Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename "soft ack" to "echo" #1103

Closed
mfalkvidd opened this issue Apr 5, 2018 · 2 comments · Fixed by #1292
Closed

Rename "soft ack" to "echo" #1103

mfalkvidd opened this issue Apr 5, 2018 · 2 comments · Fixed by #1292

Comments

@mfalkvidd
Copy link
Member

mfalkvidd commented Apr 5, 2018

The send() functions (send(), present(), sendSketchInfo(), sendBatteryLevel() and maybe more) have a parameter called ack, which often is confused with what might be called "hardware ack" (see https://forum.mysensors.org/post/34263 for one of many lengthy discussions).

Suggestion: rename the parameter to echo. That should make it harder to confuse with the hardware ack, and reflects more accurately what will actually happen:

the final destination will echo back the contents of the message, triggering the receive() function on the original node with a copy of the message, with message.isAck() set and sender/destination switched.

NOTE: We'll need to rename message.isAck() to message.isEcho() (keeping Ack in the name will be confusing) but if the isAck function is not present, existing sketches will become broken. We probably don't want to break sketches in a minor release. Maybe we can deprecate isAck and add isEcho for a smoother transition.

mfalkvidd added a commit to mfalkvidd/MySensors that referenced this issue May 30, 2019
The ack parameter is often confused with what might be called
"hardware ack"
(see https://forum.mysensors.org/post/34263 for one of many
lengthy discussions)

This change should make it harder to confuse echo with the
hardware ack, and reflects more accurately what will actually
happen.

This fixes mysensors#1103

I hope I have found all places where the naming needs to be
changed, and that I haven't inadvertently renamed any of the
*real* ack stuff.
mfalkvidd added a commit to mfalkvidd/MySensors that referenced this issue May 30, 2019
The ack parameter is often confused with what might be called
"hardware ack"
(see https://forum.mysensors.org/post/34263 for one of many
lengthy discussions)

This change should make it harder to confuse echo with the
hardware ack, and reflects more accurately what will actually
happen.

This fixes mysensors#1103

I hope I have found all places where the naming needs to be
changed, and that I haven't inadvertently renamed any of the
*real* ack stuff.
mfalkvidd added a commit to mfalkvidd/MySensors that referenced this issue May 30, 2019
The ack parameter is often confused with what might be called
"hardware ack"
(see https://forum.mysensors.org/post/34263 for one of many
lengthy discussions)

This change should make it harder to confuse echo with the
hardware ack, and reflects more accurately what will actually
happen.

This fixes mysensors#1103

I hope I have found all places where the naming needs to be
changed, and that I haven't inadvertently renamed any of the
*real* ack stuff.
mfalkvidd added a commit to mfalkvidd/MySensors that referenced this issue May 30, 2019
The ack parameter is often confused with what might be called
"hardware ack"
(see https://forum.mysensors.org/post/34263 for one of many
lengthy discussions)

This change should make it harder to confuse echo with the
hardware ack, and reflects more accurately what will actually
happen.

This fixes mysensors#1103

I hope I have found all places where the naming needs to be
changed, and that I haven't inadvertently renamed any of the
*real* ack stuff.
mfalkvidd added a commit to mfalkvidd/MySensors that referenced this issue May 30, 2019
The ack parameter is often confused with what might be called
"hardware ack"
(see https://forum.mysensors.org/post/34263 for one of many
lengthy discussions)

This change should make it harder to confuse echo with the
hardware ack, and reflects more accurately what will actually
happen.

This fixes mysensors#1103

I hope I have found all places where the naming needs to be
changed, and that I haven't inadvertently renamed any of the
*real* ack stuff.
mfalkvidd added a commit to mfalkvidd/MySensors that referenced this issue May 30, 2019
The ack parameter is often confused with what might be called
"hardware ack"
(see https://forum.mysensors.org/post/34263 for one of many
lengthy discussions)

This change should make it harder to confuse echo with the
hardware ack, and reflects more accurately what will actually
happen.

This fixes mysensors#1103

I hope I have found all places where the naming needs to be
changed, and that I haven't inadvertently renamed any of the
*real* ack stuff.
mfalkvidd added a commit to mfalkvidd/MySensors that referenced this issue May 30, 2019
The ack parameter is often confused with what might be called
"hardware ack"
(see https://forum.mysensors.org/post/34263 for one of many
lengthy discussions)

This change should make it harder to confuse echo with the
hardware ack, and reflects more accurately what will actually
happen.

This fixes mysensors#1103

I hope I have found all places where the naming needs to be
changed, and that I haven't inadvertently renamed any of the
*real* ack stuff.
@mfalkvidd mfalkvidd self-assigned this May 30, 2019
@flatsiedatsie
Copy link

Should .isAck() now be renamed to .isEcho() in node code?

@mfalkvidd
Copy link
Member Author

@flatsiedatsie yes
image

Temporary staging url for the documentation before this PR is merged: https://ci.mysensors.org/job/MySensors/job/MySensors/job/PR-1292/7/Doxygen_20HTML/MyMessage_8h.html#a8f286dd914ce8dda5a5795d9cbb74b44

mfalkvidd added a commit to mfalkvidd/MySensors that referenced this issue May 30, 2019
The ack parameter is often confused with what might be called
"hardware ack"
(see https://forum.mysensors.org/post/34263 for one of many
lengthy discussions)

This change should make it harder to confuse echo with the
hardware ack, and reflects more accurately what will actually
happen.

This fixes mysensors#1103

I hope I have found all places where the naming needs to be
changed, and that I haven't inadvertently renamed any of the
*real* ack stuff.

Thanks to @tekka007 for reviewing the changes
and catching my mistakes.
mfalkvidd added a commit that referenced this issue Jun 6, 2019
* Rename "soft ack" to echo

The ack parameter is often confused with what might be called
"hardware ack"
(see https://forum.mysensors.org/post/34263 for one of many
lengthy discussions)

This change should make it harder to confuse echo with the
hardware ack, and reflects more accurately what will actually
happen.

This fixes #1103

I hope I have found all places where the naming needs to be
changed, and that I haven't inadvertently renamed any of the
*real* ack stuff.

Thanks to @tekka007 for reviewing the changes
and catching my mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants