diff --git a/src/TypeDef.h b/src/TypeDef.h index 01697665..a6e3f245 100644 --- a/src/TypeDef.h +++ b/src/TypeDef.h @@ -308,15 +308,17 @@ /*! \brief The supplied sleep period is invalid. - The specified sleep period is shorter than the time necessary to sleep and wake the hardware, - including TCXO delay + The specified sleep period is shorter than the time necessary to sleep and wake the hardware + including TCXO delay, or longer than the maximum possible */ #define ERR_INVALID_SLEEP_PERIOD -24 /*! - \brief A specified value would cause an integer overflow + \brief The supplied Rx period is invalid. + + The specified Rx period is shorter or longer than the hardware can handle. */ -#define ERR_OVERFLOW -25 +#define ERR_INVALID_RX_PERIOD -25 // RF69-specific status codes diff --git a/src/modules/SX126x/SX126x.cpp b/src/modules/SX126x/SX126x.cpp index 44b9ee43..f57e0f75 100644 --- a/src/modules/SX126x/SX126x.cpp +++ b/src/modules/SX126x/SX126x.cpp @@ -457,20 +457,21 @@ int16_t SX126x::startReceive(uint32_t timeout) { int16_t SX126x::startReceiveDutyCycle(uint32_t rxPeriod_us, uint32_t sleepPeriod_us) { // datasheet claims time to go to sleep is ~500us, same to wake up. - // compensate for that 1ms + tcxo delay: + // compensate for that 1ms + tcxo delay. uint32_t transitionTime = _tcxoDelay_us + 1000; - if (sleepPeriod_us < transitionTime + 16) { - return(ERR_INVALID_SLEEP_PERIOD); - } sleepPeriod_us -= transitionTime; // divide by 15.625 uint32_t rxPeriodRaw = (rxPeriod_us * 8) / 125; uint32_t sleepPeriodRaw = (sleepPeriod_us * 8) / 125; - // 24 bit limit: - if ((rxPeriodRaw & 0xFF000000) || (sleepPeriodRaw & 0xFF000000)) { - return(ERR_OVERFLOW); + // 24 bit limit. Also give an error if the user passes zero values (likely unintentional) + if ((rxPeriodRaw & 0xFF000000) || (rxPeriodRaw == 0)) { + return(ERR_INVALID_RX_PERIOD); + } + // this check of the high byte also catches underflow when we subtracted transitionTime + if ((sleepPeriodRaw & 0xFF000000) || (sleepPeriodRaw == 0)) { + return(ERR_INVALID_SLEEP_PERIOD); } int16_t state = startReceiveCommon(); @@ -478,7 +479,7 @@ int16_t SX126x::startReceiveDutyCycle(uint32_t rxPeriod_us, uint32_t sleepPeriod return(state); } - byte data[6] = { + uint8_t data[6] = { (rxPeriodRaw >> 16) & 0xFF, (rxPeriodRaw >> 8) & 0xFF, rxPeriodRaw & 0xFF, (sleepPeriodRaw >> 16) & 0xFF, (sleepPeriodRaw >> 8) & 0xFF, sleepPeriodRaw & 0xFF }; diff --git a/src/modules/SX126x/SX126x.h b/src/modules/SX126x/SX126x.h index e36ab1b4..0eec2d96 100644 --- a/src/modules/SX126x/SX126x.h +++ b/src/modules/SX126x/SX126x.h @@ -508,7 +508,7 @@ class SX126x: public PhysicalLayer { \returns \ref status_codes - Note that this function assumes the unit will take 500us to change state. See datasheet [SECTION]. + Note that this function assumes the unit will take 500us + TCXO_delay to change state. See datasheet section 13.1.7, version 1.2. */ int16_t startReceiveDutyCycle(uint32_t rxPeriod_us, uint32_t sleepPeriod_us);