Skip to content

Commit

Permalink
Rename "soft ack" to echo
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@mfalkvidd
mfalkvidd committed May 30, 2019
1 parent 42a7456 commit b94eb54
Show file tree
Hide file tree
Showing 11 changed files with 146 additions and 107 deletions.
11 changes: 6 additions & 5 deletions core/MyGatewayTransport.cpp
View file
Expand Up @@ -31,13 +31,14 @@ inline void gatewayTransportProcess(void)
_msg = gatewayTransportReceive();
if (_msg.destination == GATEWAY_ADDRESS) {

// Check if sender requests an ack back.
if (mGetRequestAck(_msg)) {
// Check if sender requests an echo
if (mGetRequestEcho(_msg)) {
// Copy message
_msgTmp = _msg;
mSetRequestAck(_msgTmp,
false); // Reply without ack flag (otherwise we would end up in an eternal loop)
mSetAck(_msgTmp, true);
// Reply without echo flag, otherwise we would end up in an eternal loop
mSetRequestEcho(_msgTmp,
false);
mSetEcho(_msgTmp, true);
_msgTmp.sender = getNodeId();
_msgTmp.destination = _msg.sender;
gatewayTransportSend(_msgTmp);
Expand Down
8 changes: 7 additions & 1 deletion core/MyMessage.cpp
View file
Expand Up @@ -51,9 +51,15 @@ void MyMessage::clear(void)
miSetVersion(PROTOCOL_VERSION);
}

// TODO: Remove before v3 is released, use isEcho instead
bool MyMessage::isAck(void) const
{
return miGetAck();
return isEcho();
}

bool MyMessage::isEcho(void) const
{
return miGetEcho();
}

uint8_t MyMessage::getCommand(void) const
Expand Down
31 changes: 19 additions & 12 deletions core/MyMessage.h
View file
Expand Up @@ -250,11 +250,11 @@ typedef enum {
#define mSetCommand(_message,_command) BF_SET(_message.command_ack_payload, _command, 0, 3) //!< Set command field
#define mGetCommand(_message) ((uint8_t)BF_GET(_message.command_ack_payload, 0, 3)) //!< Get command field

#define mSetRequestAck(_message,_rack) BF_SET(_message.command_ack_payload, _rack, 3, 1) //!< Set ack-request field
#define mGetRequestAck(_message) ((bool)BF_GET(_message.command_ack_payload, 3, 1)) //!< Get ack-request field
#define mSetRequestEcho(_message,_rack) BF_SET(_message.command_ack_payload, _rack, 3, 1) //!< Set ack-request field
#define mGetRequestEcho(_message) ((bool)BF_GET(_message.command_ack_payload, 3, 1)) //!< Get ack-request field

#define mSetAck(_message,_ackMsg) BF_SET(_message.command_ack_payload, _ackMsg, 4, 1) //!< Set ack field
#define mGetAck(_message) ((bool)BF_GET(_message.command_ack_payload, 4, 1)) //!< Get ack field
#define mSetEcho(_message,_ackMsg) BF_SET(_message.command_ack_payload, _ackMsg, 4, 1) //!< Set ack field
#define mGetEcho(_message) ((bool)BF_GET(_message.command_ack_payload, 4, 1)) //!< Get ack field

#define mSetPayloadType(_message, _pt) BF_SET(_message.command_ack_payload, _pt, 5, 3) //!< Set payload type field
#define mGetPayloadType(_message) ((uint8_t)BF_GET(_message.command_ack_payload, 5, 3)) //!< Get payload type field
Expand All @@ -269,11 +269,11 @@ typedef enum {
#define miSetVersion(_version) BF_SET(version_length, _version, 0, 2) //!< Internal setter for version field
#define miGetVersion() ((uint8_t)BF_GET(version_length, 0, 2)) //!< Internal getter for version field

#define miSetRequestAck(_rack) BF_SET(command_ack_payload, _rack, 3, 1) //!< Internal setter for ack-request field
#define miGetRequestAck() ((bool)BF_GET(command_ack_payload, 3, 1)) //!< Internal getter for ack-request field
#define miSetRequestEcho(_rack) BF_SET(command_ack_payload, _rack, 3, 1) //!< Internal setter for echo request field
#define miGetRequestEcho() ((bool)BF_GET(command_ack_payload, 3, 1)) //!< Internal getter for echo request field

#define miSetAck(_ack) BF_SET(command_ack_payload, _ack, 4, 1) //!< Internal setter for ack field
#define miGetAck() ((bool)BF_GET(command_ack_payload, 4, 1)) //!< Internal getter for ack field
#define miSetEcho(_ack) BF_SET(command_ack_payload, _ack, 4, 1) //!< Internal setter for echo field
#define miGetEcho() ((bool)BF_GET(command_ack_payload, 4, 1)) //!< Internal getter for echo field

#define miSetPayloadType(_pt) BF_SET(command_ack_payload, _pt, 5, 3) //!< Internal setter for payload type field
#define miGetPayloadType() (uint8_t)BF_GET(command_ack_payload, 5, 3) //!< Internal getter for payload type field
Expand Down Expand Up @@ -380,11 +380,18 @@ class MyMessage
uint8_t getCommand(void) const;

/**
* @brief Getter for ack-flag.
* @return true if this is an ack message
* \deprecated, see isEcho
* @brief Getter for echo-flag.
* @return true if this is an echoed message
*/
bool isAck(void) const;

/**
* @brief Getter for echo-flag.
* @return true if this is an echoed message
*/
bool isEcho(void) const;

/**
* @brief Set message type
* @param type see http://korturl.nu/stupidurl
Expand Down Expand Up @@ -485,8 +492,8 @@ typedef union {

/**
* 3 bit - Command type<br>
* 1 bit - Request an ack - Indicator that receiver should send an ack back<br>
* 1 bit - Is ack message - Indicator that this is the actual ack message<br>
* 1 bit - Request an echo - Indicator that receiver should echo the message back to the sender<br>
* 1 bit - Is echo message - Indicator that this is the echoed message<br>
* 3 bit - Payload data type
*/
uint8_t command_ack_payload;
Expand Down
2 changes: 1 addition & 1 deletion core/MyOTALogging.cpp
View file
Expand Up @@ -62,7 +62,7 @@ void OTALog(uint8_t logNode, bool enableAck, const char *fmt, ... )
msg.sender = getNodeId();
msg.setDestination(logNode);
mSetCommand(msg, C_INTERNAL);
mSetRequestAck(msg, enableAck);
mSetRequestEcho(msg, enableAck);

// Send package
for (int pos = 0; pos<n; pos+=MAX_PAYLOAD) {
Expand Down
16 changes: 8 additions & 8 deletions core/MyProtocol.cpp
View file
Expand Up @@ -33,7 +33,7 @@ bool protocolSerial2MyMessage(MyMessage &message, char *inputString)
uint8_t command = 0;
message.sender = GATEWAY_ADDRESS;
message.last = GATEWAY_ADDRESS;
mSetAck(message, false);
mSetEcho(message, false);

// Extract command data coming on serial line
for (str = strtok_r(inputString, ";", &p); // split using semicolon
Expand All @@ -51,8 +51,8 @@ bool protocolSerial2MyMessage(MyMessage &message, char *inputString)
command = atoi(str);
mSetCommand(message, command);
break;
case 3: // Should we request ack from destination?
mSetRequestAck(message, atoi(str) ? 1 : 0);
case 3: // Should we request echo from destination?
mSetRequestEcho(message, atoi(str) ? 1 : 0);
break;
case 4: // Data type
message.type = atoi(str);
Expand Down Expand Up @@ -91,7 +91,7 @@ char *protocolMyMessage2Serial(MyMessage &message)
{
(void)snprintf_P(_fmtBuffer, MY_GATEWAY_MAX_SEND_LENGTH,
PSTR("%" PRIu8 ";%" PRIu8 ";%" PRIu8 ";%" PRIu8 ";%" PRIu8 ";%s\n"), message.sender,
message.sensor, mGetCommand(message), mGetAck(message), message.type,
message.sensor, mGetCommand(message), mGetEcho(message), message.type,
message.getString(_convBuffer));
return _fmtBuffer;
}
Expand All @@ -100,7 +100,7 @@ char *protocolMyMessage2MQTT(const char *prefix, MyMessage &message)
{
(void)snprintf_P(_fmtBuffer, MY_GATEWAY_MAX_SEND_LENGTH,
PSTR("%s/%" PRIu8 "/%" PRIu8 "/%" PRIu8 "/%" PRIu8 "/%" PRIu8 ""), prefix,
message.sender, message.sensor, mGetCommand(message), mGetAck(message), message.type);
message.sender, message.sensor, mGetCommand(message), mGetEcho(message), message.type);
return _fmtBuffer;
}

Expand All @@ -111,7 +111,7 @@ bool protocolMQTT2MyMessage(MyMessage &message, char *topic, uint8_t *payload,
uint8_t index = 0;
message.sender = GATEWAY_ADDRESS;
message.last = GATEWAY_ADDRESS;
mSetAck(message, false);
mSetEcho(message, false);
for (str = strtok_r(topic + strlen(MY_MQTT_SUBSCRIBE_TOPIC_PREFIX) + 1, "/", &p);
str && index < 5;
str = strtok_r(NULL, "/", &p)
Expand Down Expand Up @@ -150,8 +150,8 @@ bool protocolMQTT2MyMessage(MyMessage &message, char *topic, uint8_t *payload,
break;
}
case 3:
// Ack flag
mSetRequestAck(message, atoi(str) ? 1 : 0);
// Echo flag
mSetRequestEcho(message, atoi(str) ? 1 : 0);
break;
case 4:
// Sub type
Expand Down
41 changes: 21 additions & 20 deletions core/MySensorsCore.cpp
View file
Expand Up @@ -320,11 +320,11 @@ bool _sendRoute(MyMessage &message)
#endif
}

bool send(MyMessage &message, const bool enableAck)
bool send(MyMessage &message, const bool echo)
{
message.sender = getNodeId();
mSetCommand(message, C_SET);
mSetRequestAck(message, enableAck);
mSetRequestEcho(message, echo);

#if defined(MY_REGISTRATION_FEATURE) && !defined(MY_GATEWAY_FEATURE)
if (_coreConfig.nodeRegistered) {
Expand All @@ -338,74 +338,74 @@ bool send(MyMessage &message, const bool enableAck)
#endif
}

bool sendBatteryLevel(const uint8_t value, const bool ack)
bool sendBatteryLevel(const uint8_t value, const bool echo)
{
return _sendRoute(build(_msgTmp, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_BATTERY_LEVEL,
ack).set(value));
echo).set(value));
}

bool sendHeartbeat(const bool ack)
bool sendHeartbeat(const bool echo)
{
#if defined(MY_SENSOR_NETWORK)
const uint32_t heartbeat = transportGetHeartbeat();
return _sendRoute(build(_msgTmp, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_HEARTBEAT_RESPONSE,
ack).set(heartbeat));
echo).set(heartbeat));
#elif defined(MY_GATEWAY_FEATURE)
const uint32_t heartbeat = hwMillis();
return _sendRoute(build(_msgTmp, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_HEARTBEAT_RESPONSE,
ack).set(heartbeat));
echo).set(heartbeat));
#else
(void)ack;
(void)echo;
return false;
#endif
}



bool present(const uint8_t childSensorId, const uint8_t sensorType, const char *description,
const bool ack)
const bool echo)
{
return _sendRoute(build(_msgTmp, GATEWAY_ADDRESS, childSensorId, C_PRESENTATION, sensorType,
ack).set(childSensorId == NODE_SENSOR_ID ? MYSENSORS_LIBRARY_VERSION : description));
echo).set(childSensorId == NODE_SENSOR_ID ? MYSENSORS_LIBRARY_VERSION : description));
}

#if !defined(__linux__)
bool present(const uint8_t childSensorId, const uint8_t sensorType,
const __FlashStringHelper *description,
const bool ack)
const bool echo)
{
return _sendRoute(build(_msgTmp, GATEWAY_ADDRESS, childSensorId, C_PRESENTATION, sensorType,
ack).set(childSensorId == NODE_SENSOR_ID ? F(" MYSENSORS_LIBRARY_VERSION "): description));
echo).set(childSensorId == NODE_SENSOR_ID ? F(" MYSENSORS_LIBRARY_VERSION "): description));
}
#endif


bool sendSketchInfo(const char *name, const char *version, const bool ack)
bool sendSketchInfo(const char *name, const char *version, const bool echo)
{
bool result = true;
if (name) {
result &= _sendRoute(build(_msgTmp, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_SKETCH_NAME,
ack).set(name));
echo).set(name));
}
if (version) {
result &= _sendRoute(build(_msgTmp, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_SKETCH_VERSION,
ack).set(version));
echo).set(version));
}
return result;
}

#if !defined(__linux__)
bool sendSketchInfo(const __FlashStringHelper *name, const __FlashStringHelper *version,
const bool ack)
const bool echo)
{
bool result = true;
if (name) {
result &= _sendRoute(build(_msgTmp, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_SKETCH_NAME,
ack).set(name));
echo).set(name));
}
if (version) {
result &= _sendRoute(build(_msgTmp, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_SKETCH_VERSION,
ack).set(version));
echo).set(version));
}
return result;
}
Expand All @@ -416,9 +416,10 @@ bool request(const uint8_t childSensorId, const uint8_t variableType, const uint
return _sendRoute(build(_msgTmp, destination, childSensorId, C_REQ, variableType).set(""));
}

bool requestTime(const bool ack)
bool requestTime(const bool echo)
{
return _sendRoute(build(_msgTmp, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_TIME, ack).set(""));
return _sendRoute(build(_msgTmp, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_TIME,
echo).set(""));
}

// Message delivered through _msg
Expand Down

0 comments on commit b94eb54

Please sign in to comment.