LLCC68 check for LoRa bandwidth & spreading factor combination missing on setBandwidth?

[S5NC]

LLCC68::setSpreadingFactor prevents setting the LLCC68's spreading factor to one which isn't supported by the bandwidth, but I don't see anything preventing setting the bandwidth to one which is incompatible with the spreading factor, after the spreading factor is (initially) set.

For example, in begin set the spreading factor to 11, and bandwidth to 500 kHz. Then later change the bandwidth to 250 kHz (invalid combination for the IC). This wouldn't be caught as a specific error as the check is never ran.

LLCC68::setBandwidth without checking if the SF and BW are compatible calls SX1262::setBandwidth (no further checks in SX126X::setModulationParams).

I don't have an LLCC68 to test with. Perhaps the LLCC68 rejects the incorrect combination and RadioLib returns an error which should be managed by the program using the library, but seeing as there is a check in LLCC68::setSpreadingFactor I suppose there should be the same in LLCC68::setBandwidth. An error RADIOLIB_ERR_INVALID_SPREADING_FACTOR_BANDWIDTH_COMBINATION would be more descriptive

Note: LLCC68::setSpreadingFactor prevents the unsupported spreading factors (the ones which aren't 9, 10, or 11) by checking the range of the spreading factor in every possible bandwidth case.

The same SF-BW compatibility check could be used in LLCC68::checkDataRate however it uses bw_div2 instead of a bandwidth so would require some conversion.

Can I work on a PR? I'd like to be sure the above is how things are and should be before proceeding.

[jgromes]

You're right, both of these need to be checked. If I recall correctly, the LLCC68 device will catch these invalid combinations and return a generic SPI error, but that's far from ideal (and it's indeed the reason why the check in setSpreadingFactor is there). Additionally, seeing the mess that are LLCC68 devices from some IC lots reporting themselves as SX1262, this check definitely should be added. A PR adding the check in setBandwidth would be great!

The same SF-BW compatibility check could be used in LLCC68::checkDataRate however it uses bw_div2 instead of a bandwidth

Hmmm ... I don't actually recall why exactly does it perform this weird conversion. Just casting dr.lora.bandwidth to int should be enough.