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(telemetry): Correct A1..A2 voltage sensors value for Frsky_D/OMP #4583

Merged
merged 1 commit into from
Feb 3, 2024

Conversation

frankiearzu
Copy link
Contributor

@frankiearzu frankiearzu commented Jan 28, 2024

Fixes #4572
Tested on 2.9 branch (currently 2.9.3) as well as Main.

Summary of changes:
Make consistent the prec parameter with the Sensor definition below.

image
image

The code as it is, was creating a x10 after removing the code who was affecting other protocols when using "ratio" on PREC2 sensors (#4083). Conversion from Prec0->Prec1 means 10x.
The sensor is really a Prec1 in the definition a few lines below (2nd screenshot)

FIX:
image

@frankiearzu frankiearzu changed the title Fix for A1..A2 voltage sensors on Frsky_D telemetry Fix(telemetry): Correct A1..A2 voltage sensors value on Frsky_D/OMP telemetry broken on 2.9.3 Jan 28, 2024
@frankiearzu
Copy link
Contributor Author

frankiearzu commented Jan 28, 2024

Still think that removing/commenting the code in 2.9.3 was the right call.
image

That code affects ALL protocols (DSM/FrSkyX,etc), and looks like it was intended to fix a problem on the specific Frysky_D protocol when "ratio" is used. The "else" portion converts the prec param on the setTemetryValue (who calls getValue later) from Prec=0 to Prec=1 hiding the root cause of the problem (on Frsky_D).

Still haven't found what is the "then" part trying to fix. I can't find other sensor of Prec2 with pre-populated "ratio"

@pfeerick pfeerick changed the title Fix(telemetry): Correct A1..A2 voltage sensors value on Frsky_D/OMP telemetry broken on 2.9.3 fix(telemetry): Correct A1..A2 voltage sensors value for Frsky_D/OMP Jan 28, 2024
@pfeerick pfeerick added telemetry 📶 bug/regression ↩️ A new version of EdgeTX broke something labels Jan 28, 2024
@pfeerick pfeerick added this to the 2.9.4 milestone Jan 28, 2024
@frankiearzu
Copy link
Contributor Author

frankiearzu commented Feb 2, 2024

Was able to install old firmware in an X8R who supports D8 (jumper on Ch7-Ch8, has to be older firmware, not new V2 RX version).

Fix works as expected.

Interesting that in 2.9.2 (and before), it only gives the right value if using 'ratio'.. if no 'ratio' used (raw value from telemetry), it has the same 10x problem. It worked before since by default the 'ratio' is populated to 13.2 (about 51% of the raw reading)

@pfeerick
Copy link
Member

pfeerick commented Feb 3, 2024

Yeah, when Frsky released ACCSTv2, D8 was (no longer?) legal in the EU so you'd need ACCSTv1 firmware to use it.

I've also finally tested it on X8R in D8 mode, and can also confirm D8 A1 telemetry is restored to pre v2.9.3 behaviour - i.e. voltage is no longer 10x the value it should be (regardless of precision).

@pfeerick pfeerick merged commit db03122 into EdgeTX:main Feb 3, 2024
43 checks passed
pfeerick pushed a commit that referenced this pull request Feb 4, 2024
…or value (#4583)

Affects all Frsky_D derived telemetry (i.e. MPM => OMP)
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 telemetry 📶
Projects
None yet
Development

Successfully merging this pull request may close these issues.

telemetry voltage has wrong factor with Multiprotocol OMP
2 participants