Skip to content

Commit

Permalink
Added an algorithm to automatically compute the Rx windows parameters…
Browse files Browse the repository at this point in the history 
…. (Window symbolTimeout and Offset from downlink expected time)

Issue #196
  • Loading branch information
@mluis1
mluis1 committed Mar 17, 2017
1 parent 64cfe85 commit bc3860c
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 154 deletions.

3 comments on commit bc3860c

@GuntherSchulz
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valiant effort!

RxOffset calculation is OK
RxWindowTimeout should be MAX( ( ( 2 * DEFAULT_MIN_RX_SYMBOLS - 8 ) * tSymbol + 2 * rxError ), DEFAULT_MIN_RX_SYMBOLS * tSymbol ) ----- [RxWindowTimeout is a TIME, not a SYMBOL count]

but RxWindowSetup requires NUMBER_OF_SYMBOLS as second-last parameter

So I just added an extra field to rxConfigParams to hold number of symbols (which should be a uint16 from RxWindowSetup definition), and calculate it as (corrected) RxWindowTimeout / tSymbol, within ComputeRxWindowParameters, and pass THAT as the second-to-last parameter of RxWindowSetup.

You will then see that RxWindowTimeout per se is never used in the rest of code, so ComputeRxWindowParameters can be further optimised, and RxWindowTimeout filed can be dropped entirely

@mluis1
Copy link
Contributor Author

@mluis1 mluis1 commented on bc3860c Apr 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the current implementation of the function ComputeRxWindowParameters is correct as it provides the wanted results. And it doesn't require any change.

The result of the RxWindowTimeout computation is in number of symbols (or bytes for FSK) and it is what we want as the parameter for RxWindowSetup is also a number of symbols or (or bytes for FSK) as well as the Radio.SetRxConfig symbTimeout parameter is also the number of symbols (or bytes for FSK).

Please note that the Radio.SetRxConfig API function has also been updated in order to have the FSK timeout in number of bytes. (It wasn't used for FSK in previous versions)

/*
...
 * \param [IN] symbTimeout  Sets the RxSingle timeout value
 *                          FSK : timeout number of bytes
 *                          LoRa: timeout in symbols
...
 */
void SX1272SetRxConfig( RadioModems_t modem, uint32_t bandwidth,
                         uint32_t datarate, uint8_t coderate,
                         uint32_t bandwidthAfc, uint16_t preambleLen,
                         uint16_t symbTimeout, bool fixLen,
                         uint8_t payloadLen,
                         bool crcOn, bool FreqHopOn, uint8_t HopPeriod,
                         bool iqInverted, bool rxContinuous );

The column symbols shown in table below is the result of the formula for the RxWindowTimeout. We have also controlled the results by measuring with an oscilloscope that all values are correct.

computrxwindowparameters

@GuntherSchulz
Copy link

@GuntherSchulz GuntherSchulz commented on bc3860c Apr 11, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mluis1

Yes, your calcs are correct, i was more on the point that the name implies a time, not a symbol count. Please consider a relevant name change to avoid confusion for others. I believe strongly in naming things after what they actually represent, to make code maintenance easier.

You'll note that my calcs produce identical numbers to yours, since in mine I pre-multiply your value by tSymbol and assign it to RxWindowTimeout. Then post-divide THAT value by tSymbol to get my "new parameter / field" which hold the symbol count (same value as yours).

Note 3 things

  1. In LoRa mode, both SX1272 and SX1276 support at most a 10-bit symbol count split over REG_LR_MODEMCONFIG2 and REG_LR_SYMBTIMEOUTLSB. So we should be prudent and hard-limit the output in SX127<2/6>SetRxConfig to (2^10-1) on the LoRa branch BEFORE copying the bits into the registers, lest a higher number effectively rolls over mod 2^10 (i.e. if the number is too big, at least set the biggest possible value, rather than wrapping around to a small value).

  2. Is FSK mode, we use timers for a timeout, so the upper limit there will be based on the actual HW timer implementation, so using the value as-is should be fine in the generic case (numbers grow as as SysTimingError/4, so at SysTimingError >= 79 ms we start having problems with software, but hardware timing errors of 79ms would have been catastrophic, so should not really happen in practice).

  3. We should really have the native type of the (possibly renamed) RxWindowTimeout be uint16_t, since in LoRa case (2^10-1) is the realizable limit in HW, and in the FSK case (2^16-1) would be around 5.08 seconds - MORE than enough. Also, as previously mentioned, SX127<2/6>SetRxConfig takes a uint16_t as a parameter here, so why waste RAM for storage and do calcs on unnecessarily long types?

Thanks for the feedback, and keep rocking the good work! :)

P.S. I was aware of the change of Radio.SetRxConfig, thanks!

Please sign in to comment.