MySensors
MySensors copied to clipboard
mysensors
Remove send from within receive() in examples
Some examples, for example https://www.mysensors.org/build/dimmer uses send() from within receive().
send() should not be called from within receive(). In the dimmer example, calling send() will probably not break anything, but when people merge sketches they leave the send calls in receive which can lead to unintentional results.
Affected examples
- DimmableLEDActuator
- DimmableLight
- PingPongSensor
Reason 1: Using send will overwrite the message buffer, so the message that receive() thinks it is currently processing will be lost.
In this code example, when the second if clause is processed, message will no longer contain the original message if the first clause matched.
void receive(const MyMessage &message)
if (message.type==V_LOCK_STATUS) {
send(...)
}
else if (message.type==V_TEMP) {
// do something with the temp
}
}
Reason 2: Using send can trigger another receive which can trigger another send which can trigger another receive so we get infinite recursion (until the mcu crashes).
Not sure if there are other reasons.
In the code example, only one of the if else block code bodies will be executed, right?
On reason 2: if another receive will be triggered in the node, let's call it N, depends on if the receiver, let's call it R, of the send message will send another message to N to receive, right? So if R expects a message, eg an echo of what R previously sent to N, R should know that it shouldn't risk causing infinite recursion by sending a message again and so on.
Basically, I think there are legitimate use cases where we want to send from receive. Is there some other suggestion how to handle that use case?
No, both ifs can be executed. That's the problem.
Yes, if the sketch developer takes sufficient care, calling send from within receive can work. But we've seen a few forum threads where people don't understand the consequences and get themselves into trouble. That's why I think the basic examples shouldn't call send from within receive.
Maybe storing messages in a send buffer that the loop will pick from? Populate the buffer from receive.
No, both ifs can be executed. That's the problem.
There's an else after the if, that means the first wasn't executed if the else is matched, as far as I know. I don't get it.
If send() manipulates the message buffer in an unfortunate way, then when the compiler evaluates the else if it could very well match.
Pretty old issue, but still open, so I leave a comment :)
send uses reference to a message:
bool send(MyMessage &message, const bool requestEcho)
and _sendRoute which is used internally also uses reference:
bool _sendRoute(MyMessage &message)
so it basically means that you shouldn't use the same MyMessage object as received from receive function. And it is impossible, because receive parameter is const.
Next internal function is gatewayTransportSend, which also uses reference to provided MyMessage object:
bool gatewayTransportSend(MyMessage &message)
Next we have the last internal function protocolMyMessage2Serial, where we have some buffer (_fmtBuffer), but it is not used anywhere else.
The function gatewayTransportSend uses standard Serial.print, which operate on dedicated buffer for sending (_tx_buffer[SERIAL_TX_BUFFER_SIZE]), so it can't interfere with incoming data.
My conclusion is that there is no contraindication to using the function send inside receive.
