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 mapping of Gadegtbridge GPS data and adjust event name #2434

Merged
merged 3 commits into from Jan 5, 2023

Conversation

LukasEdl
Copy link
Contributor

Hi everyone i found some bugs in the handling of the gadgetbridge gps data.

The current state

If we retrieve the gps data from the Gadegtbridge app, we receive a JSON object with the different values. The object has a key "long" which contains the longitude gps value. The received object will then be published under the "gps" event on the global Bangle object.

Changes in this PR

To prevent breaking of code if you start using the gps data provided by Gadgetbridge, i adjusted the event name from "gps" to "GPS" and added a mapper to map the longitude value from "long" to "lon" so that it is similar to a gps object, which you wouuld receive if you use the normal gps chip of the banglejs device.

@thyttan
Copy link
Collaborator

thyttan commented Dec 30, 2022

You should probably update version number in metadata.json and ChangeLog :)

add change log message
@LukasEdl
Copy link
Contributor Author

I updated the version number and added the changes to the change log.

@halemmerich
Copy link
Collaborator

Why map the long/lon value on the bangle instead of just changing the event send by GB? The code was only included/usable in nightlies so far, I think we could just correct this at the source and be done with it.

@gfwilliams
Copy link
Member

Thanks! I guess ideally Gadgetbridge would be changed to send lon not long but we have to modify this anyway to send the correct event type, so for now I think this is fine.

My concern is I wonder how well this whole thing was tested now ;)

@gfwilliams gfwilliams merged commit e4c5795 into espruino:master Jan 5, 2023
@halemmerich
Copy link
Collaborator

Maybe it would make sense to hold off on a new GB release until this has been fixed. The current code would remove the correct value the moment GB would be changed to send lon as key by overwriting it by the no longer existing value of long.

I have tested a bit with manually doing the changes of this PR. It seems to work OK, turning GPS on at the Bangle instantly shows the location indicator on my phone and I am getting GPS events on the watch. The main problem I currently see is the phone showing a notification for sending GPS to two devices and it not switching off when the bangle stops using GPS. Maybe we need a timeout after a few seconds in boot code that checks if _PWR contains GPS and sends the power off message
to GB otherwise?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants