betaflight / betaflight

Open Source Flight Controller Firmware
GNU General Public License v3.0
8k stars 2.86k forks source link

Use the cached value of useDshotTelemetry to ensure consistent runtime use if dshot_bidir is changed #13589

Closed SteveCEvans closed 2 weeks ago

SteveCEvans commented 3 weeks ago

Fixes #13581

motorConfig()->dev.useDshotTelemetry was being referenced many times in the code and if the value was changed then uninitialised filters would be used etc. This PR uses the value cached upon initialisation of DSHOT, stored in useDshotTelemetry.

github-actions[bot] commented 3 weeks ago

Do you want to test this code? You can flash it directly from Betaflight Configurator:

WARNING: It may be unstable. Use only for testing!

HThuren commented 3 weeks ago

@SteveCEvans you have 2 instances of motorConfig()->dev.useDshotTelemetry in dshot.c line 162 - initDshotTelemetry, maybe they also need replace with useDshotTelemetry

SteveCEvans commented 2 weeks ago

No, as initDshotTelemetry() is only called from init() just after the config is read, and not thereafter. The issue here was caused by motorConfig()->dev.useDshotTelemetry being read when dshot_bidir changed the value after everything was initialised. Reading the value during initialisation is OK. Note that rpmFilterInit() is called not just on initial initialisation, but also when the profile command is issued so that was causing an issue.

HThuren commented 2 weeks ago

No, as initDshotTelemetry() is only called from init() just after the config is read, and not thereafter. The issue here was caused by motorConfig()->dev.useDshotTelemetry being read when dshot_bidir changed the value after everything was initialised. Reading the value during initialisation is OK. Note that rpmFilterInit() is called not just on initial initialisation, but also when the profile command is issued so that was causing an issue.

Right, it is a job to get into and understand the source code :-)