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): temperature UNIT conversion when temperature has decimals #4728

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

frankiearzu
Copy link
Contributor

@frankiearzu frankiearzu commented Mar 12, 2024

Fixes #4723

Summary of changes:

  1. Fixes converting from C to F (and vice versa) when the Temperature has decimal places.
  2. Optimization: Remove loop to do 10^n, and use a table "Power10[n]" to change from one Prec to another.
  3. Optimization: Only do the conversion if needed, i.e, Different UNIT or different Prec
  4. Move the exception of not displaying "Precision" on 3 different GUIs when sensor is UNIT_FARENHEITH to a single share code by all GUIs. TelemetrySensor::isPrecConfigurable() is only used for this UI purpose, nowhere else.

Left the Precision not been configurable when sensor is UNIT_FARENHEITH, (same behavior as before). I think this is because is already granular enough compared to UNIT_CELSIOUS.

Examples: Using LemonRX Gen2 who has a temperature sensor embeded.

image
image
NOTE: Temp was increasing while taking the screenshots.. that's why the little difference.
image

1. Properly convert between C/F regardless of if they have decimal places or not.

2. Move the check to disable editing Prec to a single point.
isPrecConfigurable()
Also add the exception for FARENHEIGHT to isPrecConfigurable(), so is a single point since all the different GUIS use it
@frankiearzu frankiearzu changed the title Tel temperature fix fix(telemetry): Temperature UNIT conversion when temperature has decimals. Mar 12, 2024
@frankiearzu
Copy link
Contributor Author

frankiearzu commented Mar 12, 2024

@pfeerick DONE! sometimes im surprised of finding this bugs after the code been like that for so many years, Was more exposed recently that in DSM automatically convers the units to METRIC or IMPERIAL (for display) depending on the radio configuration (something missing in many other protocols.. will add a chore for this).

@pfeerick
Copy link
Member

pfeerick commented Mar 12, 2024

Yeah, some the bugs leave you scratching your head... clearly no-one used those combinations + thought to report it + someone actually bothered to try and fix it 🤪 ... it's like the "Adjust GV" long enter bug that was fixed the other day... it's been there some 6-7 years for all X7 navigation radios... yet it survived all this time... 🤪

@pfeerick pfeerick changed the title fix(telemetry): Temperature UNIT conversion when temperature has decimals. fix(telemetry): temperature UNIT conversion when temperature has decimals Mar 19, 2024
@pfeerick pfeerick added this to the 2.10 milestone Mar 24, 2024
@pfeerick pfeerick self-requested a review March 24, 2024 09:31
@pfeerick
Copy link
Member

pfeerick commented Apr 3, 2024

Thanks for this... sorry it took so long to get around to it. I see what you mean with the decimal conversion i.e. 0.0 and 0.00 was completely wrong. LGTM with AFHDS2A and DSMX receivers (with internal/additional temperature sensors).

@pfeerick pfeerick merged commit e3f2914 into EdgeTX:main Apr 3, 2024
44 checks passed
@frankiearzu frankiearzu deleted the tel-temperature-fix branch April 3, 2024 14:47
ThomasKuehne pushed a commit to ThomasKuehne/edgetx that referenced this pull request Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Telemetry: Temperature UNIT conversions not working when temperature has decimals
2 participants