Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task done
ivanb123github opened this issue Apr 28, 2023 · 57 comments
Closed
1 task done

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

ivanb123github opened this issue Apr 28, 2023 · 57 comments
Labels
bug 🪲 Something isn't working
Milestone

Comments

@ivanb123github
Copy link

Is there an existing issue for this problem?

  • I have searched the existing issues

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 ivanb123github added bug 🪲 Something isn't working triage Bug report awaiting review / sorting labels Apr 28, 2023
@ivanb123github
Copy link
Author

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

@ivanb123github ivanb123github changed the title widgets vs. sensors widgets vs. sensors (TX16s/FS-IA6B ) Apr 29, 2023
@ivanb123github ivanb123github changed the title widgets vs. sensors (TX16s/FS-IA6B ) widgets vs. sensors (TX16s/FS-IA6B FW2.8.3) Apr 29, 2023
@richardclli
Copy link
Collaborator

Yes the sensors calculation changes. You can refer to:

#3386

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

Did you use a MPM?

@ivanb123github
Copy link
Author

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
Copy link

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
Copy link
Collaborator

richardclli commented May 1, 2023

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:

if (type == 0xAA)
value = (packet[3] << 8) | packet[2];
else
value = (packet[5] << 24) | (packet[4] << 16) | (packet[3] << 8) | packet[2];

The code in v2.8.2 is:

if (type == 0xAA)
value = (packet[3] << 8) | packet[2];
else
value = (packet[6] << 24) | (packet[5] << 16) | (packet[4] << 8) | packet[3];

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
Copy link

Steini63 commented May 1, 2023

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
Copy link
Author

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
Copy link
Member

pfeerick commented May 2, 2023

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
Copy link

Steini63 commented May 3, 2023

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
Copy link
Collaborator

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

@richardclli
Copy link
Collaborator

richardclli commented May 10, 2023

  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:

  • INr4-GYB with build in gyro
  • Gmr with external gyro FS-GY01
    i.e. No changes can be applied to this.
  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
Copy link

Steini63 commented May 10, 2023

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 richardclli removed the triage Bug report awaiting review / sorting label May 11, 2023
@richardclli richardclli added this to the 2.9 milestone May 11, 2023
@richardclli
Copy link
Collaborator

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
Copy link
Author

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
Copy link

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

@Steini63
Copy link

Steini63 commented May 12, 2023

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
Copy link
Author

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
Copy link
Member

pfeerick commented May 13, 2023

@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 #3582

@ivanb123github
Copy link
Author

I have now downloaded FW Peter Feerick as advised. Thank you. It's easier and more accurate. But nothing changed. #3543 (comment)
screen-2023-05-13-133348

@Steini63
Copy link

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
Copy link
Member

pfeerick commented May 14, 2023

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
Copy link

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 #3583) isn't part of 2.8.4 ?

@pfeerick
Copy link
Member

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.

@Steini63
Copy link

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

Frank

@ajjjjjjjj
Copy link
Contributor

It is almost the same source missing some refactorings.

@richardclli
Copy link
Collaborator

richardclli commented May 27, 2023

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
Copy link
Collaborator

richardclli commented Jul 27, 2023

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).

{FLYSKY_SENSOR_GPS, 2, STR_SENSOR_GPS, UNIT_GPS_LATITUDE, 0, 1, 4, true},
{FLYSKY_SENSOR_GPS, 3, STR_SENSOR_GPS, UNIT_GPS_LONGITUDE, 0, 5, 4, true},

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: #3582

@ivanb123github
Copy link
Author

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
Copy link
Collaborator

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

@ivanb123github
Copy link
Author

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
Copy link
Collaborator

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

@ivanb123github
Copy link
Author

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

@ivanb123github
Copy link
Author

Yes I delete the sensors and rediscover

@richardclli
Copy link
Collaborator

richardclli commented Aug 2, 2023

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
Copy link
Author

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

@ivanb123github
Copy link
Author

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.

@3djc
Copy link
Collaborator

3djc commented Aug 2, 2023

image

image

Seems to work ok

@pfeerick
Copy link
Member

pfeerick commented Aug 3, 2023

Note that this is in relation to Flysky telemetry (not Frsky) - so not necessarily indicative of an issue with that setting (other than it not recognising the values as being valid GPS data).

@richardclli
Copy link
Collaborator

Note that this is in relation to Flysky telemetry (not Frsky) - so not necessarily indicative of an issue with that setting (other than it not recognising the values as being valid GPS data).

Ideally, since the sensors are not produced by Flysky (3rd party). It will be nice to make use of the Frsky GPS sensors formatting, but I cannot figure out a way to generic way to work this out. So leave it as RAW so it is readable.

Maybe the sensors need a refactoring. ^.^

@pfeerick
Copy link
Member

pfeerick commented Aug 3, 2023

Maybe the sensors need a refactoring. ^.^

Surely not! 🤪 🤣

@ivanb123github
Copy link
Author

for 3dj:

  1. For what type of TX and RX.
  2. For what type of sensor.

@frankiearzu
Copy link
Contributor

Looks like this might have been solve by #4062

@pfeerick
Copy link
Member

Indeed it does look like #4080 seem to have finally been put this issue to rest, thanks to your efforts and those who helped in #4062 to test it.

@pfeerick pfeerick modified the milestones: 2.9.x, 2.9.2 Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants