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

widgets vs. sensors (TX16s/FS-IA6B FW2.8.3) #3543

Closed ivanb123github closed 1 year ago

ivanb123github commented 1 year ago

Is there an existing issue for this problem?

What part of EdgeTX is the focus of this bug?

Build system

Current Behavior

I tried FW 2.8.3 for TX16s with RX FS-IA6B and it maps telemetry data to widgets incorrectly. This is a telemeter. data from the sensor of my construction. So I reverted back to FW 2.8.2 and everything works OK again.

Expected Behavior

see above

Steps To Reproduce

  1. upload firmware 2.8.3 from https://github.com/EdgeTX/edgetx/releases/tag/v2.8.3

Version

2.8.2

Transmitter

Radiomaster TX16S / TX16SMK2

Operating System (OS)

No response

OS Version

No response

Anything else?

No response

ivanb123github commented 1 year ago

I'm sorry, this is not a move to widgets, but sensor values from the receiver. I specify, see the pictures. Issue3543-230429.zip

richardclli commented 1 year ago

Yes the sensors calculation changes. You can refer to:

https://github.com/EdgeTX/edgetx/pull/3386

It is aligned with Flysky official products NV14/EL18, and the PR is created by Flysky engineers.

Did you use a MPM?

ivanb123github commented 1 year ago

Yes, i use mpm 1.3.3.20 . For my sensor construction, I use the library according to https://github.com/adis1313/iBUSTelemetry-Arduino.

Steini63 commented 1 year ago

I can also confirm new bugs related to iBus telemetry introduced with EdgeTx 2.8.3:

  1. The sensor "Cels" (ID 04) shows the values too high by a factor of 10. This was still ok in version 2.8.2.

  2. At the 4-byte sensors (ID 80 - ID 8x) apparently the byte order got mixed up. Byte0 (LSB) has always the value 4, Byte1 has the value of Byte0, Byte2 the value of Byte3 and so on.

  3. Furthermore there is an old bug concerning the iBus telemetry: The sensors for latitude and longitude (ID 80 and 81) are still displayed (in degrees) as a comma value with only two digits. This is useless for position determination. If I could wish for something, I would wish for the same display as with s.Port telemetry, hoping to be able to use all the tools provided for this.

I use a Radioking TX18S and a Radiomaster TX16S, both with the internal MPM 1.3.3.20.

Regards Frank

richardclli commented 1 year ago

I can also confirm new bugs related to iBus telemetry introduced with EdgeTx 2.8.3:

1. The sensor "Cels" (ID 04) shows the values too high by a factor of 10. This was still ok in version 2.8.2.

2. At the 4-byte sensors (ID 80 - ID 8x) apparently the byte order got mixed up. Byte0 (LSB) has always the value 4, Byte1 has the value of Byte0, Byte2 the value of Byte3 and so on.

3. Furthermore there is an old bug concerning the iBus telemetry:
   The sensors for latitude and longitude (ID 80 and 81) are still displayed (in degrees) as a comma value with only two digits. This is useless for position determination.
   If I could wish for something, I would wish for the same display as with s.Port telemetry, hoping to be able to use all the tools provided for this.

I use a Radioking TX18S and a Radiomaster TX16S, both with the internal MPM 1.3.3.20.

Regards Frank

I can verify the reason of point 2 is due to the PR given from flysky, and from the code it seems it is logical for the change they made, they fixed a bug as packet[0] = type, packet[1] = instance and data starts from packet[2] onwards, see the current code:

https://github.com/EdgeTX/edgetx/blob/53f172bb0703156db1cedaeb381bdbb88b8a9e9d/radio/src/telemetry/flysky_ibus.cpp#L306-L310

The code in v2.8.2 is:

https://github.com/EdgeTX/edgetx/blob/03e80fb7dffc3b103eddfc90d085f0e4794a8063/radio/src/telemetry/flysky_ibus.cpp#L162-L165

I think maybe there are some 3rd party software out there that implements something based on the bug in OpenTX/EdgeTX and now the bug is fixed by Flysky, finally, those software are no longer working properly.

I think we may need more information about all the software/firmware that you use to trace the simple question "Why the code is like this ?" in the first place.

Steini63 commented 1 year ago

Sensor and software I use is the successor of this: https://www.franksteinberg.de/BLHeliTelemetryFeeder.html The new device provides GPS data and is not yet released. I don't think it's relevant though. Just like the Arduino library used by ivanb123github, this only handles the communication between sensor and receiver. This is mandatory and cannot be changed. But the bug concerns the communication between transmitter module (MPM) and transmitter (EdgeTx).

I suspect the following: In the communication between FlySky transmitter module and transmitter, in the case of 4-byte payload data, a superfluous byte was transmitted by the transmitter module (packet[2]). This was fixed in an update to the FlySky transmitter module and the EdgeTx code was adapted to it. Since the MPM had to imitate this superfluous byte, the detected error occurs.

Was there an update for a FlySky transmitter module?

Frank

ivanb123github commented 1 year ago

Fixes #3196 - explains the problem of transmitting sensor values on TX EL18 with AFHDS 3 protocol I don't know that AFHDS 3 protocol is implemented in multi TX16s. I can't judge at this moment what the difference in data transmission is between AFHDS 3 and AFHDS 2. But I think fix#3196 in 2.8.3 was for AFHDS 3 protocol and therefore affected correctness for TX16s with AFHDS 2. Ivan

pfeerick commented 1 year ago

I don't know that AFHDS 3 protocol is implemented in multi TX16s.

It isn't - only AFHDS2A. However, since iBus telemetry is common to both versions, and thus also the MPM, if there are any fixes that make things work properly with official Flysky sensors and spec, this can then expose any bugs in MPM telemetry processing that were masked in previous code, or present on the OTX/ETX code all this time. That's not to say there aren't new ones now... this is a living project after all 🤣

Steini63 commented 1 year ago

Ok, how should the compatibility for the 4-byte data be restored?

In the current state no usable values are transferable with the MPM. The MPM is the backbone for OpenTx and EdgeTx transmitters.

iBus telemetry is used for many DIY projects and finally OpenXSensor (https://github.com/mstrens/oXs_on_RP2040) can speak iBus protocol.

Frank

richardclli commented 1 year ago

Let me talk to Flysky and see if there are any resolutions.

richardclli commented 1 year ago
  1. The sensor "Cels" (ID 04) shows the values too high by a factor of 10. This was still ok in version 2.8.2.

ID 04 is used by Flysky gyro sensor that used in:

  1. At the 4-byte sensors (ID 80 - ID 8x) apparently the byte order got mixed up. Byte0 (LSB) has always the value 4, Byte1 has the value of Byte0, Byte2 the value of Byte3 and so on.

This change can be reverted, should not affect other Flysky product. (Need Flysky to test and confirm)

  1. Furthermore there is an old bug concerning the iBus telemetry: The sensors for latitude and longitude (ID 80 and 81) are still displayed (in degrees) as a comma value with only two digits. This is useless for position determination.

Let me see if this can be fixed.

Steini63 commented 1 year ago

I would like to thank you very much for your efforts.

  1. ... ID 04 is used by Flysky gyro sensor that used in:
    • INr4-GYB with build in gyro
    • Gmr with external gyro FS-GY01 i.e. No changes can be applied to this.

This should only be a minor issue. DIY'ers can easily fix it on the sensor side. I changed to ID 03 (shown as "A3") to transmit battery voltage.

  1. ... This change can be reverted, should not affect other Flysky product. (Need Flysky to test and confirm)

Sounds good! Fixing issue 2 would be very important. I sincerely hope that it works out.

  1. ... Let me see if this can be fixed.

A format that the yaapu widget understands would be great.

Frank

richardclli commented 1 year ago

I would like to thank you very much for your efforts.

  1. ... ID 04 is used by Flysky gyro sensor that used in:
  • INr4-GYB with build in gyro
  • Gmr with external gyro FS-GY01 i.e. No changes can be applied to this.

This should only be a minor issue. DIY'ers can easily fix it on the sensor side. I changed to ID 03 (shown as "A3") to transmit battery voltage.

  1. ... This change can be reverted, should not affect other Flysky product. (Need Flysky to test and confirm)

Sounds good! Fixing issue 2 would be very important. I sincerely hope that it works out.

  1. ... Let me see if this can be fixed.

A format that the yaapu widget understands would be great.

Frank

I have made a PR, please test if it works for you.

ivanb123github commented 1 year ago

I have made a PR, please test if it works for you.

Sorry, I'm new here and I don't know the exact wording. But if I understand correctly, you made the change in 2.9. nightly I tested on my TX16s FW 2.9 and the change was not noticeable. So I compared the flysky_ibus* files for 2.9 and 2.8.3 and they are identical. Where did you make the change?

Steini63 commented 1 year ago

Same problem here. Do we have to compile on our own? I never did but I'm willing to try.

Steini63 commented 1 year ago

Ok, I managed to compile https://github.com/EdgeTX/edgetx/tree/fix-3543 via gitpod.

Result: screen-2023-05-12 So issue 2. seems to be resolved but no progress for issue 3.

My position is Latitude 52.4006896° Longitude 9.6357594°

Sensor 29 and 30 shows the values if I use ID 0088 and 0089, shown as RAW data. Sensor 31 (GPS) and 32 (GPS) shows the values if I transmit the same data using ID 0080 and 0081 (intended for latitude and longitude by FlySky).

Frank

ivanb123github commented 1 year ago

I also managed to translate the fix as well. VerzeFix And here is the result I adjusted the settings for sensor 10 in TX16s, originally it was set as sensor11. These sensors are the same value as sensor 13,14 to RX via ibus. screen-2023-05-13-091235 originally senzor11 screen-2023-05-13-091334 and widget screen-2023-05-13-093102 In the DIY version of the sensor, I adjusted the value for the sensor to 11, and the result in the TX16s is still the same. Ivan

pfeerick commented 1 year ago

@ivanb123github @Steini63 Just for future reference:

To access the automatic builds for a PR, first open up the PR page. Under the PR title, you should see a series of tabs (Comments, Commits, Checks, Files Changed). Click on the Checks tab. Then if you are after the radio firmware, click on the link on the left side mentioning firmware. If you want a Companion/Simulator build, pick the one that suits your operating system. Then, under the 'Artifacts' heading near the bottom of the page will be a link to the zip file containing either the firmware or companion package.

In this case, the PR in question is https://github.com/EdgeTX/edgetx/pull/3582

ivanb123github commented 1 year ago

I have now downloaded FW Peter Feerick as advised. Thank you. It's easier and more accurate. But nothing changed. https://github.com/EdgeTX/edgetx/issues/3543#issuecomment-1546614299 screen-2023-05-13-133348

Steini63 commented 1 year ago

Same here. I got a bin the advised way but no change.

The 0°00'0.0N 0°00'0.0E format seem to be shown after a sensor-search wich I did not yesterday.

screen-2023-05-13 "3008" at sensor 19 means 3D-fix and 8 satellites in use

Frank

pfeerick commented 1 year ago

Just as a general note, whenever any changes are made around telemetry / telemetry sensors, it is advisable to reset the telemetry sensors... i.e. delete all and discover new... as any settings changes such as what units are used are not going to be changed from those that are configured.

From the looks of things clearly there is more work to be done 😆 btw, 2.8.4 is about to get released (finally, since the windows companion builds are working again), but it will have no bearing on this - it is going to be primarily only for x9d+2019 users due to a firmware overflow issue.

Steini63 commented 1 year ago

2.8.4 is about to get released (finally, since the windows companion builds are working again), but it will have no bearing on this

Does that mean, even the fix for issue 2. (and presumably https://github.com/EdgeTX/edgetx/issues/3583) isn't part of 2.8.4 ?

pfeerick commented 1 year ago

No, because it's still a work in progress. If we manage to fix more of this, it can be part of 2.8.5 (or 2.9, depending on timeline). 2.8.4 was delayed due to issues with the build system.

richardclli commented 1 year ago

So if the 4 bytes comes out properly now? The format of ID80 and ID81, I just give a try to select a proper format type, not sure if it works anyway. Do you have any reference ID for Frksy sensors that you have mentioned? The definitions on how to format a sensor values are coded in mapping tables, for Frksy ones:

https://github.com/EdgeTX/edgetx/blob/e4a281973ba2f8412ffcbc89c7f2f3aac994fcf5/radio/src/telemetry/frsky_sport.cpp#L24-L103

And for Flysky ibus:

https://github.com/EdgeTX/edgetx/blob/70d4d62f4924bd4af17ec927fa08739c87ad107f/radio/src/telemetry/flysky_ibus.cpp#L147-L197

So I need to know which types are you expected for GPS: e.g. UNIT_DEGREE, UNIT_GPS, UNIT_RAW, etc.

Steini63 commented 1 year ago

Do you have any reference ID for Frksy sensors that you have mentioned? The definitions on how to format a sensor values are coded in mapping tables, for Frksy ones:

For Frsky I use ID 0x0809 as mentioned in https://github.com/EdgeTX/edgetx/blob/e4a281973ba2f8412ffcbc89c7f2f3aac994fcf5/radio/src/telemetry/frsky_defs.h:

#define GPS_LONG_LATI_FIRST_ID    0x0800
#define GPS_LONG_LATI_LAST_ID     0x080F

Unit is "GPS" according to https://github.com/EdgeTX/edgetx/blob/e4a281973ba2f8412ffcbc89c7f2f3aac994fcf5/radio/src/telemetry/frsky_sport.cpp { GPS_LONG_LATI_FIRST_ID, GPS_LONG_LATI_LAST_ID, 0, STR_SENSOR_GPS, UNIT_GPS, 0 },

Result: screen-2023-05-15

This format would be my wish also for FlySky because I assume that the compatibility is the highest. This is also how I understand the FlySky table:

{ AFHDS2A_ID_GPS_LAT,               STR_SENSOR_GPS,           UNIT_GPS,               7 },  // 4 bytes signed WGS84 in degrees * 1E7 
{ AFHDS2A_ID_GPS_LON,               STR_SENSOR_GPS,           UNIT_GPS,               7 },  // 4 bytes signed WGS84 in degrees * 1E7 

Of course, an output in degrees with sufficient decimal places would also be useful. But there seems to be no suitable 4-byte data format for this at the moment.

Frank

richardclli commented 1 year ago

@Steini63 Ah, these 2 are quite different, Frsky combines the LAT and LON into single value, while Flysky mapping is not. Try change it to UNIT_DEGREE to see if it works. I have updated the PR.

ivanb123github commented 1 year ago

Now use FW tx16s-576d284.bin My application sends the value 15.92443 to RX ibus-sensor (RX ia6b) for GPS_LON But the TX16S will only display 15.92. It should be 15.92443 correctly. Why not? Where is the value truncated to only two decimal places?

Steini63 commented 1 year ago

I downloaded the recent PR-firmware, run it, delete all sensors, discover new. The difference is, that the degree sign "°" is displayed, but still two decimal places only.

So it's the same as ivanb123github describes.

richardclli commented 1 year ago

So it is the formatter problem, I am thinking if it is possible to define a new sensor that work like the Frsky that merge lat and long together instead.

Steini63 commented 1 year ago

On the sensor side, I can say that the raw data is sent in the same format for both Frsky and FlySky, namely in the said "4 bytes signed WGS84 in degrees * 1E7". So also the gps-module sends them in ublox binary format.

The difference is that with Frsky latitude and longitude must be transmitted alternately with the same ID. With FlySky there are separate IDs (0080 and 0081).

Frank

ivanb123github commented 1 year ago

I would like to know how the GPS is displayed in the TX16S from the FS-CGPS01 sensor (original flysky) connected to the i-bus-sensor in the RX IA6B? Is it possible to find out somewhere?

Steini63 commented 1 year ago

I found a video on youtube; title "Flysky ST8 FS-CGPS01 GPS set up." https://www.youtube.com/watch?v=moBQjrCO_GQ

Screenshot: Screenshot_FS-ST8

Frank

ivanb123github commented 1 year ago

More precisely, I would like to know how the mentioned sensor communicates with the RX IA6b. In general, see the attached Flysky-Ibus-en.txt. I couldn't find anywhere eg for (AC FRAME)[ID][inst][size] see ibus_flysky.cpp. Is there a full description of the flysky IBUS-sensor available somewhere? Flysky-Ibus-en.txt.

Steini63 commented 1 year ago

During the development of my converter I recorded the communication between sensors and ia6b using a terminal program. The converter reads the telemetry data sent from a BLHeli32-ESC and converts it to iBus telemetry. In the picture below a screenshot of the hammer terminal is shown. The hex-values on the left are from the terminal, the explanatory text I added with an image editor. Since only 2-byte values are transferred, it is perhaps not quite what you are looking for, but it may still help:

iBus-telemetry

Download Hammer-Terminal: https://www.der-hammer.info/pages/terminal.html

Frank

ivanb123github commented 1 year ago

Likewise, I have a terminal. But I used the Saleae logic analyzer. Otherwise, flysky_ibus.cpp analyzes two types of sensors - AA and AC. ....... // AC type telemetry with multiple values in one packet AFHDS2A_ID_GPS_FULL = 0xFD, ................... What sensor is it? It is not defined in flysky_ibus.cpp. How does it communicate with RX? There was mention of a new sensor for UNIT_GPS. Shouldn't he be an AC type? Therefore, I would be interested in the protocol specification from flysky, where I would find more detailed information. Unfortunately, I couldn't find them anywhere. Note. I think what was done in PR3543 is sufficient for me at least, as I use the telemetry data only as information. The question is if it fits the professional flysky sensors. Ivan

ajjjjjjjj commented 1 year ago

Copied my comment from PR: The root issue here is that FlySky "PREC7" is not really supported. In convertTelemetryValue() value is divided to lower precision from 7 to 2 effectively making: value /= 100000.

The easy fix is to lower precision from 7 to 2, so at least value is not truncated.

ajjjjjjjj commented 1 year ago

Regarding GPS lat/lon, I have fixed it this way: https://github.com/OpenI6X/opentx/pull/354/files

Not the perfect solution but relatively simple and reuses GPS formatting methods.

Steini63 commented 1 year ago

Sounds promising. How does this solution come from OpenI6X to EdgeTx?

Frank

ajjjjjjjj commented 1 year ago

It is almost the same source missing some refactorings.

richardclli commented 1 year ago

Umm.... not a good idea to include protocol specific dependencies in UI code, should only depends on the telemetry information alone. Let me see if I can work out a more generic solution.

richardclli commented 1 year ago

The difference is that with Frsky latitude and longitude must be transmitted alternately with the same ID. With FlySky there are separate IDs (0080 and 0081).

https://github.com/EdgeTX/edgetx/blob/7cd9577528292e450ac91f2ec0a2df59e7fddaa1/radio/src/telemetry/flysky_nv14.cpp#L79-L80

I found something interesting, I tried to setup the sensors similar to these configurations, please check if this works for you.

@Steini63 Please build and test this PR: https://github.com/EdgeTX/edgetx/pull/3582

ivanb123github commented 1 year ago

I am sending a comparison of telemetry data in version 2.8.2 and PR3582 and a test program (Gps-iBUS-FS-IA6b.zip.) GpsTx16s210-PR3582values GpsTx16s210-PR35822info GpsTx16s282info GpsTx16s282values Gps-iBUS-FS-IA6b.zip

richardclli commented 1 year ago

I changed the units to RAW, see if this gives better outcome.

ivanb123github commented 1 year ago

I am sending a comparison of telemetry data in version 2.8.2 and PR3582 and a test program (Gps-iBUS-FS-IA6b.zip.) GpsTx16s210-PR3582values GpsTx16s210-PR35822info GpsTx16s282info GpsTx16s282values Gps-iBUS-FS-IA6b.zip

richardclli commented 1 year ago

Did you delete the sensors and rediscover? It seems nothing is changed.

ivanb123github commented 1 year ago

richardcll - Does this mean I should install new FW for PR?

ivanb123github commented 1 year ago

Yes I delete the sensors and rediscover

richardclli commented 1 year ago

Yes, I changed the PR, you need to rebuild and reinstall the firmware. Your firmware is still Jul-27. And what I change is to use RAW, which should be the same as ID85/86 in your testing screens.

ivanb123github commented 1 year ago

yes GPS the same as ID85/86, but they should have a decimal point. Example 50.3608704, 17.53578752

ivanb123github commented 1 year ago

Notice. In TX16 settings I can choose between NMEA or DMS format for GPS. For nmea it is 50.3608556 respectively 15.9244172 and for DMS it should be 50°21'39.080 respectively 15°55'27.902 . I tried changing the settings but there was no change for DMS.