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(ibus): Separate lat long sensors merged to single GPS sensor #4080

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

frankiearzu
Copy link
Contributor

@frankiearzu frankiearzu commented Sep 21, 2023

Update code to populate single sensor GPS with Lat/Long together

Fixes #4062
Code is safe to be released with 2.9.1

Summary of changes:
This is for FlySky_IBUS protocol.
The original code was poulating GPS latitude and longitude in two instances of the "GPS" sensor. The bug fix asked to give them a different name (GLat.GLon), but the standard in many other protocols in EdgeTX is to have both in a single sensor.
It is displayed correctly in the UI and Widgets. Also the logs will inteact properly with goggle maps.

Testing:
Since i don't have that sensor, worked together with the bug reporter by providing firmware with the changes for him to test for me. After a few iteration, we got it right!!

image

@richardclli
Copy link
Collaborator

Looks good, finally have someone managed to convert the value to GPS format, yeah.

@frankiearzu
Copy link
Contributor Author

If someone test this with a GPS sensor different than the home buikd oXs, yould you please post a picture of the sensors with the "Show instanceID" flag ON.. I have a suspision that oXs is not handling the instanceID correctly on all GPS sensors, but don't have the equipment to test, In other protocols, all GPS sensors will have same instanceID (different sensorID of couse).

@frankiearzu
Copy link
Contributor Author

frankiearzu commented Sep 26, 2023

@pfeerick the change that @philmoz did on Main to save memory conflicted with this PR.
Do you want me to keep this one for 2.9.1 and create another PR for 2.10 (main)?

@pfeerick
Copy link
Member

This one is already targeting main (2.10), and trivial to correct for both directions, so no need to change anything. Given it looks like there will be few users with GPS, we'll probably have to go with this as is until someone with a working native Flysky GPS says it doesn't work (I stress working because as the GPS I got a few months ago didn't work).

frankiearzu and others added 2 commits September 26, 2023 13:35
Update code to populate single sensor GPS with Lat/Long together
@pfeerick pfeerick changed the title FlySky IBUS GPS sensor Fix fix(ibus): Separate lat long sensors merged to single GPS sensor Sep 26, 2023
@pfeerick pfeerick added bug 🪲 Something isn't working telemetry 📶 labels Sep 26, 2023
@pfeerick pfeerick added this to the 2.9.1 milestone Sep 26, 2023
@pfeerick pfeerick merged commit 6f40ea4 into EdgeTX:main Sep 26, 2023
@frankiearzu frankiearzu deleted the flysky_ibus_GPS_Fix branch September 26, 2023 05:36
@ivanb123github
Copy link

2.9.1 - For TX16s and a homemade GPS sensor (arduino) the display is fine as it should be
note. If there was an option to set the font size in the widgets, it would be even better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working telemetry 📶
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GPS telemetry variable names (lat/lon both named "GPS")
4 participants