search

ratgdo / mqtt-ratgdo

ratgdo via mqtt
GNU General Public License v2.0
85 stars 20 forks source link

Variable might be too small for value in transmit() #45

Closed p-jean closed 10 months ago

p-jean commented 11 months ago

In transmit(), if millis() is nearly the same as lastRX, then the value of txDelayLen will be smaller than expected due to wrap-around with the uint8_t type.

void transmit(byte* payload, unsigned int length){
                ...
        uint8_t txDelayLen;
        byte tempPayload[1];

        txDelayLen = (lastRX + 275) - millis();
        delay(txDelayLen);
                ...
}
PaulWieland commented 10 months ago

This isn't an issue.

edrikk commented 10 months ago

Hi @PaulWieland Although probably not hit, his (base-)point is still valid. That is: millis() returns an "unsigned long" which is 32 bits (definitely is on Arduinos) while uint8_t is an unsigned 8-bit integer.

It's not a bad idea to define txDelayLen (and any other variable that is working with/against millis() as "unsigned long" similar to this recent commit in the homekit firmware.