Rename messageId into messageCounter #9688

kghost · 2021-09-14T13:14:44Z

Problem

MessageCounter is the official name in the specs.

Change overview

This PR is generated by following commands:

git sed -f g messageId messageCounter
git sed -f g MessageId MessageCounter
git sed -f g MsgId MessageCounter
git sed -f g msgId messageCounter
git sed -f g msgID messageCounter

Testing

examples/minimal-mdns/client.cpp

src/app/zap-templates/zcl/data-model/silabs/ami.xml

src/messaging/ReliableMessageContext.cpp

src/messaging/ReliableMessageContext.h

github-actions · 2021-09-14T15:36:54Z

Size increase report for "gn_qpg-example-build" from 29e6bcd

File	Section	File	VM
chip-qpg6100-lighting-example.out	.text	36	36

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
.debug_str,0,240
.text,36,36
.strtab,0,22
.debug_line,0,4
.shstrtab,0,-2
[Unmapped],0,-36

github-actions · 2021-09-14T16:06:41Z

Size increase report for "esp32-example-build" from 29e6bcd

File	Section	File	VM
chip-temperature-measurement-app.elf	.flash.rodata	40	40
chip-bridge-app.elf	.flash.rodata	32	32
chip-shell.elf	.flash.rodata	40	40
chip-lock-app.elf	.flash.text	68	68
chip-lock-app.elf	.flash.rodata	32	32
chip-all-clusters-app.elf	.flash.rodata	40	40

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.debug_str,0,250
.flash.rodata,40,40
.strtab,0,22
.debug_line,0,2
.shstrtab,0,-2
[Unmapped],0,-40

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-bridge-app.elf and ./pull_artifact/chip-bridge-app.elf:

sections,vmsize,filesize
.debug_str,0,250
.flash.rodata,32,32
.strtab,0,22
.debug_line,0,2
.shstrtab,0,2
[Unmapped],0,-32

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_str,0,230
.flash.rodata,40,40
.strtab,0,22
.debug_line,0,2
.shstrtab,0,-2
[Unmapped],0,-40

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
.debug_str,0,250
.flash.text,68,68
.flash.rodata,32,32
.strtab,0,22
.xt.prop._ZNK4chip4SpanIhE7SubSpanEjj,0,12
.xt.lit._ZNK4chip4SpanIhE7SubSpanEjj,0,8
.debug_loc,0,4
.debug_line,0,2
.shstrtab,0,2
[Unmapped],0,-100

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_str,0,250
.flash.rodata,40,40
.strtab,0,10
.debug_line,0,2
.shstrtab,0,-2
[Unmapped],0,-40

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

github-actions · 2021-09-14T16:06:54Z

Size increase report for "nrfconnect-example-build" from 29e6bcd

File	Section	File	VM
chip-lock.elf	rodata	40	36
chip-shell.elf	rodata	32	36

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_str,0,240
rodata,36,40
.strtab,0,22
.debug_line,0,4
.debug_loc,0,4
.shstrtab,0,2

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_str,0,240
rodata,36,32
.strtab,0,22
.debug_line,0,4
.shstrtab,0,-2
.debug_loc,0,-4

bzbarsky-apple · 2021-09-14T15:12:08Z

src/messaging/ReliableMessageContext.cpp

@@ -150,36 +150,36 @@ uint64_t ReliableMessageContext::GetActiveRetransmitTimeoutTick()
 *  @note
 *    This message is part of the CHIP Reliable Messaging protocol.
 *
- *  @param[in]    AckMsgId         The msgId of incoming Ack message.
+ *  @param[in]    AckMessageCounter         The messageCounter of incoming Ack message.


Pre-existing, somewhat, but this is really confusing. This is not the actual message counter of the ack message. It's the message counter being acked. In spec terms, this is the "Acknowledged Message Counter" field.

Please do a followup to clean up the documentation here, or let me know once this PR merges and I can do it.

bzbarsky-apple · 2021-09-14T15:19:28Z

src/lib/core/CHIPError.h

@@ -2177,7 +2177,7 @@ using CHIP_ERROR = ::chip::ChipError;
 * @def CHIP_ERROR_MESSAGE_ID_OUT_OF_WINDOW
 *
 * @brief
- *   The message id of the received message is out of receiving window
+ *   The message counter of the received message is out of receiving window
 */
 #define CHIP_ERROR_MESSAGE_ID_OUT_OF_WINDOW                        CHIP_CORE_ERROR(0xc7)


Followup: Should change the name of this error too?

bzbarsky-apple · 2021-09-14T15:22:27Z

src/messaging/ReliableMessageContext.cpp

@@ -150,36 +150,37 @@ uint64_t ReliableMessageContext::GetActiveRetransmitTimeoutTick()
 *  @note
 *    This message is part of the CHIP Reliable Messaging protocol.
 *
- *  @param[in]    AckMsgId         The msgId of incoming Ack message.
+ *  @param[in]    ackMessageCounter         The messageCounter of incoming Ack message.


This is the "Acknowledged Message Counter" of the incoming message, not the "Message Counter", right?

Pre-existing; please either do a followup to fix this documentation to be less confusing or let me know when this merges and I will.

bzbarsky-apple · 2021-09-14T15:42:42Z

src/messaging/ReliableMessageMgr.h

@@ -151,11 +151,11 @@ class ReliableMessageMgr
     *
     *  @param[in]    rc        A pointer to the ExchangeContext object.
     *
-     *  @param[in]    msgId     message ID which has been acked.
+     *  @param[in]    messageCounter     message ID which has been acked.


Suggested change

* @param[in] messageCounter message ID which has been acked.

* @param[in] messageCounter message counter which has been acked.

boring-cyborg bot added app examples lib transport labels Sep 14, 2021

kghost force-pushed the message-counter branch from effb14e to 3eae1d1 Compare September 14, 2021 13:26

kghost marked this pull request as ready for review September 14, 2021 14:06

pullapprove bot requested review from andy31415, bzbarsky-apple, chrisdecenzo, Damian-Nordic, hawk248, jepenven-silabs and msandstedt September 14, 2021 14:06

pullapprove bot added the review - pending label Sep 14, 2021

msandstedt approved these changes Sep 14, 2021

View reviewed changes

Damian-Nordic requested changes Sep 14, 2021

View reviewed changes

examples/minimal-mdns/client.cpp Outdated Show resolved Hide resolved

src/app/zap-templates/zcl/data-model/silabs/ami.xml Outdated Show resolved Hide resolved

kghost force-pushed the message-counter branch from 3eae1d1 to d7c4e2d Compare September 14, 2021 14:38

Damian-Nordic approved these changes Sep 14, 2021

View reviewed changes

src/messaging/ReliableMessageContext.cpp Outdated Show resolved Hide resolved

src/messaging/ReliableMessageContext.h Outdated Show resolved Hide resolved

kghost force-pushed the message-counter branch from d7c4e2d to a6231d2 Compare September 14, 2021 15:12

Rename messageId into messageCounter

e8c3ec1

kghost force-pushed the message-counter branch from a6231d2 to e8c3ec1 Compare September 14, 2021 15:13

andy31415 approved these changes Sep 14, 2021

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Sep 14, 2021

andy31415 added the technical review - DONE label Sep 14, 2021

andy31415 merged commit fa0ef98 into project-chip:master Sep 14, 2021

bzbarsky-apple approved these changes Sep 14, 2021

View reviewed changes

kghost mentioned this pull request Sep 15, 2021

Rename messageId into messageCounter #9714

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename messageId into messageCounter #9688

Rename messageId into messageCounter #9688

kghost commented Sep 14, 2021 •

edited

Loading

github-actions bot commented Sep 14, 2021

github-actions bot commented Sep 14, 2021

github-actions bot commented Sep 14, 2021

bzbarsky-apple Sep 14, 2021

bzbarsky-apple Sep 14, 2021

bzbarsky-apple Sep 14, 2021

bzbarsky-apple Sep 14, 2021

	* @param[in] messageCounter message ID which has been acked.
	* @param[in] messageCounter message counter which has been acked.

Rename messageId into messageCounter #9688

Rename messageId into messageCounter #9688

Conversation

kghost commented Sep 14, 2021 • edited Loading

Problem

Change overview

Testing

github-actions bot commented Sep 14, 2021

github-actions bot commented Sep 14, 2021

github-actions bot commented Sep 14, 2021

bzbarsky-apple Sep 14, 2021

Choose a reason for hiding this comment

bzbarsky-apple Sep 14, 2021

Choose a reason for hiding this comment

bzbarsky-apple Sep 14, 2021

Choose a reason for hiding this comment

bzbarsky-apple Sep 14, 2021

Choose a reason for hiding this comment

kghost commented Sep 14, 2021 •

edited

Loading