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

fix: Some AFHDS2A/AFHDS3 GPS/ALT telemetry handling issues #3582

Merged
merged 6 commits into from
Aug 4, 2023
Merged

Conversation

richardclli
Copy link
Collaborator

@richardclli richardclli commented May 11, 2023

Fixes (partially) #3543
Fixes #3583

@ajjjjjjjj
Copy link
Contributor

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.

@pfeerick
Copy link
Member

pfeerick commented Jun 2, 2023

So where are things at with this PR now?

@richardclli
Copy link
Collaborator Author

Need more work for this. Need somebody to help to compare the output of the same sensor connect via different equipments.

@pfeerick
Copy link
Member

pfeerick commented Jun 15, 2023

Need somebody to help to compare the output of the same sensor connect via different equipments.

As in? AFHDS2 vs AFHDS3 receiver? Native vs MPM?

@richardclli
Copy link
Collaborator Author

richardclli commented Jun 15, 2023

Need somebody to help to compare the output of the same sensor connect via different equipments.

As in? AFHDS2 vs AFHDS3 receiver? Native vs MPM?

Yes, there are a few paths in the code:

  1. AFHDS3 receiver connected to PL18 with Flysky firmware.
  2. AFHDS3 receiver connected to EdgeTX radios with IRM301 or FRM303.
  3. AFHDS2 receiver connected to EdgeTX NV14 with internal AFHDS2 module.
  4. AFHDS2 receiver connected to EdgeTX radios with MPM.

All I want to do is to align the sensor outputs in different paths.

@pfeerick
Copy link
Member

pfeerick commented Jun 15, 2023

You're lucky... the PL18 still has Flysky fw on it atm, so I can do 1,3,4 easily enough, and need to do the https://github.com/EdgeTX/edgetx/wiki/Flysky-FRM303-Mod-for-TX16S for some of the other PRs and stuff anyway, so will do that tomorrow (thus 2, with FRM303). My MPM TX16S with failed externalaccessmod gets a second chance 🤭

@pfeerick
Copy link
Member

pfeerick commented Jun 17, 2023

So after some fiddling around, these are the results (with the current state of this PR, and either FS-iA6B or FTr12B RX, the same FS-CAT01 sensor mostly stationary on the desk, attempting to have similar power on and run times to minimise self-heat variability... was done in the order of this list which may explain some variance):

AFHDS3 receiver connected to PL18 with Flysky firmware.
Started at 30m alt, drifted down to 25m over the course of a minute or two. Pressure not reported at all.

AFHDS3 receiver connected to EdgeTX radios with IRM301 or FRM303.
Was able to bind, but only got two telemetry sensors? With another PR, I got all the sensors, and it was fairly stable around alt 25m, 1016.2 pressure

AFHDS2 receiver connected to EdgeTX NV14 with internal AFHDS2 module.
Started around 25m, wandered down to 20m alt, 1016.11 pressure

AFHDS2 receiver connected to EdgeTX radios with MPM.
Wandering between 19m to 22m, 1015.7 pressure

So, IMO, pressure was as consistent as it's going to be with these sensors (when reported). Altitude consistently went down the longer the sensor was connected, so it may be related to self-heat if the temperature is (edit) not factored in as part of the calculation. While from start to end it's a variance of 10m, overall it's not excessively great a variance. I'm going to try the FRM303 again with a rebased version of this branch to see if telemetry was broken when this PR was started. edit: I did that with no change, but also realised you must have rebased this recently as it was only a few commits behind. Will try again tomorrow with a nightly to try and pin it down further.

@richardclli
Copy link
Collaborator Author

So after some fiddling around, these are the results (with the current state of this PR, and either FS-iA6B or FTr12B RX, the same FS-CAT01 sensor mostly stationary on the desk, attempting to have similar power on and run times to minimise self-heat variability... was done in the order of this list which may explain some variance):

AFHDS3 receiver connected to PL18 with Flysky firmware. Started at 30m alt, drifted down to 25m over the course of a minute or two. Pressure not reported at all.

AFHDS3 receiver connected to EdgeTX radios with IRM301 or FRM303. Was able to bind, but only got two telemetry sensors? With another PR, I got all the sensors, and it was fairly stable around alt 25m, 1016.2 pressure

AFHDS2 receiver connected to EdgeTX NV14 with internal AFHDS2 module. Started around 25m, wandered down to 20m alt, 1016.11 pressure

AFHDS2 receiver connected to EdgeTX radios with MPM. Wandering between 19m to 22m, 1015.7 pressure

So, IMO, pressure was as consistent as it's going to be with these sensors (when reported). Altitude consistently went down the longer the sensor was connected, so it may be related to self-heat if the temperature is factored in as part of the calculation. While from start to end it's a variance of 10m, overall it's not excessively great a variance. I'm going to try the FRM303 again with a rebased version of this branch to see if telemetry was broken when this PR was started. edit: I did that with no change, but also realised you must have rebased this recently as it was only a few commits behind. Will try again tomorrow with a nightly to try and pin it down further.

Alt is calculated with pressure alone.

@richardclli
Copy link
Collaborator Author

I think I may need to work out some more tweaks for the gps readings in this PR. Almost forgotten about this PR

@pfeerick pfeerick changed the title Fix widgets vs. sensors (TX16s/FS-IA6B FW2.8.3) and updated meaningfu… fix: Some AFHDS2A/AFHDS3 GPS/ALT telemetry handling issues Aug 4, 2023
@pfeerick pfeerick added the bug/regression ↩️ A new version of EdgeTX broke something label Aug 4, 2023
@pfeerick pfeerick merged commit c2c9c5a into main Aug 4, 2023
36 checks passed
@pfeerick pfeerick deleted the fix-3543 branch August 4, 2023 05:30
pfeerick pushed a commit that referenced this pull request Aug 4, 2023
* Fix widgets vs. sensors (TX16s/FS-IA6B FW2.8.3) and updated meaningful units.

* Updated the GPS sensors to degree unit.

* Fixed a bug in AFHDS2 Alt decoding.

* Fixed a typo.

* Update GPS sensors format.

* Change Long/Lat unit to RAW.
pfeerick pushed a commit that referenced this pull request Aug 4, 2023
* Fix widgets vs. sensors (TX16s/FS-IA6B FW2.8.3) and updated meaningful units.

* Updated the GPS sensors to degree unit.

* Fixed a bug in AFHDS2 Alt decoding.

* Fixed a typo.

* Update GPS sensors format.

* Change Long/Lat unit to RAW.
@MikeBland
Copy link

I needed to add Flysky altitude telemetry processing to er9x (for the ATMEGA devices). I've used this code, but had problems due to the uint64_t arithmetic. Looking closely at the code the 19-bit pressure value is shifted left 16 places (needing more than 32 bits), then divided by the SeaLevelPressure (101320).
I notice that 101320 is divisible by 8, so instead of shifting the pressure left by 16 bits I've shifted it left by 13 bits, then divided by (SeaLevelPressure/8), giving the same result, but only needing a 32-bit unsigned integer.
I also notice that the table is indexed by "index" and "index"+1. Since index may be 255, index+1 would reference past the end of the table. This may not be a real world problem as it only occurs if the altitude is above 10000 metres!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/regression ↩️ A new version of EdgeTX broke something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

V 2.8.3 Flysky FS-CAT01 altitude sensor
4 participants