search

EdgeTX / edgetx

EdgeTX is the cutting edge open source firmware for your R/C radio
https://edgetx.org
GNU General Public License v2.0
1.6k stars 340 forks source link

Trainer mode slave/jack initial end channel CH0 #3236

Closed elecpower closed 1 year ago

elecpower commented 1 year ago

Is there an existing issue for this problem?

What part of EdgeTX is the focus of this bug?

Transmitter firmware

Current Behavior

Setting a model training mode to Slave/Jack displays an invalid end channel number

Expected Behavior

Valid end channel number

Steps To Reproduce

  1. Launch simulator from Companion
  2. click MDL button
  3. go to Setup
  4. click Trainer
  5. change mode to Slave/Jack
  6. note end channel CH0
  7. edit end channel
  8. click + and changes the value to CH4, click again and CH5
  9. clicking - repeatedly and the value does not go below CH4
  10. leave the end channel at say CH7
  11. change the mode to OFF
  12. click RTN
  13. click Trainer
  14. change the mode to Slave/Jack and the end channel is the old value eg CH7

Version

Other (Please specify below)

Transmitter

Radiomaster TX16S / TX16SMK2

Anything else?

Branch: main Screenshot from TX16S libsim screenshot_tx16s_23-02-22_07-33-07

pfeerick commented 1 year ago

Interesting... this doesn't happen if you 1) create a new model in the simulator or 2) on the actual transmitter... in both cases it initialises to CH8.

Why is Companion writing channelsCount: -8 when it's set to OFF? i.e. there is a bug on the transmitter side as bounds checking isn't being done, but Companion seems to be setting the wrong value as well. The TX16S is writing channelsCount: 0 and defaulting to 8CH... need to see if that is specific to that or global - if so, either that needs to change or the Companion default of 4CH needs to change to 8CH

i.e. Companion default output

trainerData:
  mode: OFF
  channelsStart: 0
  channelsCount: -8
  frameLength: 0
  delay: 0
  pulsePol: 0
elecpower commented 1 year ago

From memory this is an old binary 'special' to save space and save an initialisation step so the interpreted value is channelsCount + 8 thus -8 + 8 = 0 and 0 + 8 is 8. However if OFF makes no sense to store -8. Likely some gremlins in the OTX B&W code to consider too. IMO this is one of those cases where we should rectify the sins of the past.