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.62k stars 340 forks source link

Telemetry Sensor SubID shows wrong ID #2678

Closed ParkerEde closed 1 year ago

ParkerEde commented 2 years ago

Is there an existing issue for this problem?

What part of EdgeTX is the focus of this bug?

Transmitter firmware

Current Behavior

When I look at the telemetry SubIDs in the radio and also in the "Companion Simulator", they are always displayed 1 too low (n-1). Example: on a FrSky SR6 the RSSI is transmitted by default via ID F101 SubID 25. In the radio on the telemetry page it says 24, but in the Companion programming window it says the correct SubID 25. It's the same with an ACCST based RX like X8R

Correct in Companion programming window: image

Wrong in Simulator and radio firmware: image and image

Expected Behavior

Simulator and radio firmware should display the correct SubID

Steps To Reproduce

  1. MDL
  2. Enter Telemtrie Page
  3. Look at ID for RSSI
  4. validate this ID with the programming page in Companion of the same received Modeldata.

Version

Nightly (Please give date/commit below)

Transmitter

FrSky X10 Express / X10S Express (ACCESS)

Anything else?

2.8.0 selfbuild be86b59d

raphaelcoeffic commented 2 years ago

@ParkerEde I believe this is the opposite way: the radio shows you the real sensor ID and Companion adds 1 to it when displaying.

pfeerick commented 2 years ago

There also overlaps with #2665 and the older #1650

ParkerEde commented 2 years ago

@ParkerEde I believe this is the opposite way: the radio shows you the real sensor ID and Companion adds 1 to it when displaying.

Hi @raphaelcoeffic , I am very sure that it is displayed incorrectly on the radio. Here is an example of how the UniSens-E sends its data (ID20) and the radio displays ID 19. image image image

In addition, the FRSky sensors like ALT and VSpd which transmit on ID1 are displayed as ID0 on the radio.

raphaelcoeffic commented 2 years ago

@ParkerEde you can believe me, the ID displayed on radio is what is used on the wire, I checked it ;-) If others tend to refuse to use ID 0 and prefer to add 1 to it, it's not really our issue, is it?

raphaelcoeffic commented 2 years ago

By the way, the other ID displayed (0x53) is more correct. The real sensor ID is then 0x53 & 0x1F = 0x13 (the upper 3 bits are just a filler). And as far as I'm concerned, 0x13 = 19, not 20.

gagarinlg commented 2 years ago

I agree, we should use the IDs as they are on the wire/in the air and not add 1 artificially

ParkerEde commented 2 years ago

@raphaelcoeffic Thank you for the investigation and explanation. Would you then recommend adjusting the ID in Companion to match the radio?

raphaelcoeffic commented 2 years ago

@raphaelcoeffic

Thank you for the investigation and explanation. Would you then recommend adjusting the ID in Companion to match the radio?

Indeed, we should stay consistent and make Companion display the ID as stored, instead of artificially adding 1 to it. @elecpower thoughts?

elecpower commented 2 years ago

For a bit of history, the shift was added in OTX to radios and Companion back in https://github.com/opentx/opentx/pull/6865 ETX Companion should align with radio so PR coming soon.

ParkerEde commented 1 year ago

This issue is now also fixed with PR #3401, so I'm closing it here.