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

android: interfacing GB activity tracks #2819

Merged
merged 1 commit into from Mar 11, 2024

Conversation

thyttan
Copy link
Collaborator

@thyttan thyttan commented Jun 13, 2023

WIP

Companion PR to Gadgetbridge repo: https://codeberg.org/Freeyourgadget/Gadgetbridge/pulls/3153

Related conversation on the espruino forum: https://forum.espruino.com/conversations/387110/#17000044

  • indicate on the bangle that fetching is being done? widget?

@thyttan thyttan changed the title android: WIP interfacing GB activity tracks android: interfacing GB activity tracks Jun 13, 2023
apps/android/boot.js Outdated Show resolved Hide resolved
@gfwilliams
Copy link
Member

gfwilliams commented Jun 14, 2023

Hi - I can see why you might want this, but also I guess you could just send the relevant JS code over from gadgetbridge rather than including it in this file? Maybe it does make more sense in here though...

I'm not 100% sure what you mean about iterating over the alphabet, but I think you probably need to list all available files with code like in https://github.com/espruino/BangleApps/blob/master/apps/recorder/app.js#L99 and then go through that list - that seems safest.

Also sending that data is going to take a long time - maybe 30 sec or more per file - so I wonder if we should actually split up the read somehow, at least per-track...

@thyttan
Copy link
Collaborator Author

thyttan commented Jun 14, 2023

Yes, I'm just trial-and-erroring my way to a solution.

I had forgotten about sending code to run from Gadgetbride - if you suggest it then that's probably better 👍

@gfwilliams
Copy link
Member

if you suggest it then that's probably better

Not always! I think it retrospect what you've done is better - at least the code is readily viewable rather than buried in Gadgetbridge.

Maybe you need one handler to get a list of available tracks, and another to read a track? Then Gadgetbridge can decide which one to read.

@thyttan thyttan force-pushed the android-gb-activity-tracks branch 4 times, most recently from d6ec9a7 to 5876509 Compare June 16, 2023 23:39
@thyttan thyttan force-pushed the android-gb-activity-tracks branch 3 times, most recently from 22d5bdb to 5f7eb7c Compare June 26, 2023 21:31

"listRecs": function() { // GB({t:"listRecs", id:"20230616a"})
let recs = require("Storage").list(/^recorder\.log.*\.csv$/,{sf:true}).map(s => s.slice(12, 21));
recs = recs.slice(recs.indexOf(event.id)); // FIXME: Currently does not handle if the last fetched log was deleted on the watch, so that there won't be any entry corresponding to `event.id`
Copy link
Collaborator Author

@thyttan thyttan Jun 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to do this with findIndex() instead, to find the first log that is newer than the provided id. The checking function should check the date and appended letter. This way we also don't need the shift() function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that'd be nicer I guess - not that the current solution is bad though

@thyttan thyttan force-pushed the android-gb-activity-tracks branch 2 times, most recently from 7d9c944 to 290a6f2 Compare July 4, 2023 22:30
@thyttan thyttan force-pushed the android-gb-activity-tracks branch from b9f3a5e to 20ce944 Compare July 12, 2023 23:49
@thyttan
Copy link
Collaborator Author

thyttan commented Nov 7, 2023

@gfwilliams

I'm getting crashes in Gadgetbridge with the current state of my code for activity tracks. The crashes are a consequence of empty entries for GPS readings which I currently don't handle.

The log I'm testing with was set to take readings every second, has gps data from the watch and heartrate data from a bluetooth strap. There are some inconsistencies with the logged time where e.g. the same second will be logged twice. A somewhat anonymized version of the log is attached.

recorder.log2023MMDDa.csv

Now I'm wondering how I should continue. Are these two things (inconsistencies with logging time and empty gps entries) bugs to work out on Bangle.js or is it something I should handle when processing the data in Gadgetbridge?

I guess it's more sensible to log with maybe 5 or 10 second intervals for readings instead. But since it's possible to log down to every second we should handle it some way, on the bangle or in gadgetbridge.

@gfwilliams
Copy link
Member

That's an interesting one... Do you think you could have been swapping between apps which might have caused the duplicate lines? I just checked with a period of 1s here and it works fine for me - if you look in "recorder.json" what is period set to?

Occasional dups might be expected since we're rounding the time to the nearest second, but it's a surprise it's so often since writeLog which writes the lines always gets called from setInterval which if period was 1 should be every second, so you'd hope the log period would be absolute fastest of one/sec.

You'd expect empty GPS lines every so often - in this case it'll be because data is being written faster than the GPS is supplying it, but in other cases maybe the GPS would lose tracking for a few minutes and you want other data to be written to the log.

But, as far as I'm concerned: Nothing the watch does should be able to crash Gadgetbridge so really any code in Gadgetbridge needs to be resilient enough to handle badly formatted data and throw it away/error.

@thyttan
Copy link
Collaborator Author

thyttan commented Nov 7, 2023

Ok, thanks! 🙂

@thyttan
Copy link
Collaborator Author

thyttan commented Nov 7, 2023

if you look in "recorder.json" what is period set to?

Well, I've changed the setting since I recorded the log. But I had set the period to 1 second in the app.

Occasional dups might be expected since we're rounding the time to the nearest second, but it's a surprise it's so often since writeLog which writes the lines always gets called from setInterval which if period was 1 should be every second, so you'd hope the log period would be absolute fastest of one/sec.

Looking at it again, the pattern seems to be that the log "jumps" past a second without recording it, followed by two readings with the same reported second. E.g.:

1697638056
1697638057
1697638059 <-- should have ended with 58???
1697638059
1697638060

@gfwilliams
Copy link
Member

Ahh, that does make slightly more sense though. What app are you running while doing the recording? Just the recorder app or something else?

When you say setInterval(..., 1000) Espruino can't always run your code exactly every 1000ms as some other code might be running at the same time, so instead it makes sure that it evens things out - for example if some other code is running and is busy for 100ms, it'll end up scheduling at 1100ms, but then the next time it'll schedule 900ms after that, to ensure that even if it wasn't at exactly 1000ms, two iterations take 2000ms, and in a minute you'd have exactly 60 iterations

So based on that, this behaviour isn't unexpected at all, especially if you have an app running that is keeping the Bangle busy for some non-trivial amount of time.

I can see it's frustrating but I'm not entirely sure what we can do if we're keeping the timestamp rounded... While we could just increment the timestamp by 1 there's a chance that it might drift off over time.

I guess one option is just to say that if we're using a 1000ms period, we output the timestamp with 1 decimal place?

@thyttan
Copy link
Collaborator Author

thyttan commented Nov 8, 2023

Ahh, that does make slightly more sense though. What app are you running while doing the recording? Just the recorder app or something else?

Either the recorder app or run+. Don't remember if I used some other app for another purpose or not.

I can see it's frustrating but I'm not entirely sure what we can do if we're keeping the timestamp rounded... While we could just increment the timestamp by 1 there's a chance that it might drift off over time.

It's a bit of a pain when doing division for speed/cadence/etc calculations in Gadgetbridge, yes 😛. But can be handled.

I guess one option is just to say that if we're using a 1000ms period, we output the timestamp with 1 decimal place?

For my project here that sounds nice! I guess someone could look at their logs and wonder why not all periods are the same. But they would already be experiencing this '5,6,7,9,9' behavior - which is worse.

Another solution is to have the shortest allowed period be 2 seconds.

@thyttan
Copy link
Collaborator Author

thyttan commented Nov 13, 2023

@gfwilliams

I could do a PR for either:

  1. Add one decimal place precision for 1 sec periods.
    or
  2. Make 2 seconds the lower bound for recorder periods.

Which would you prefer?

@gfwilliams
Copy link
Member

Thanks! Let's do 1 decimal place - I bet someone will complain if we remove the 1 second option.

Just a thought: ideally for 1 second we'd just output data as soon as we got the GPS reading (assuming we were supposed to log GPS - we might not be). That would help to stop us getting the blank GPS entries and would make sure we didn't miss any GPS readings too. It is possible to configure the GPS to give data at a higher rate, but I think that's fine - it's rare anyone would do that, and if they did they might even like being able to record all the data they get

@thyttan
Copy link
Collaborator Author

thyttan commented Nov 14, 2023

Just a thought: ideally for 1 second we'd just output data as soon as we got the GPS reading (assuming we were supposed to log GPS - we might not be). That would help to stop us getting the blank GPS entries and would make sure we didn't miss any GPS readings too.

If we do this,

  • should the recorder not start logging until the GPS starts pushing updates?

or

  • should it initially log with ~1 sec periods until GPS kicks in?

@gfwilliams
Copy link
Member

Argh - good point! I think it has to start logging every second if there's no GPS!

However, thinking about it I'm pretty sure we still get a GPS event from the GPS even when there's no fix, so we can still write our log line when that happens and we'll be ok?

@thyttan thyttan force-pushed the android-gb-activity-tracks branch 4 times, most recently from d5ecada to 544b3fd Compare February 17, 2024 12:23
@thyttan thyttan force-pushed the android-gb-activity-tracks branch 4 times, most recently from bdd7789 to cc376d4 Compare February 29, 2024 23:46
@thyttan
Copy link
Collaborator Author

thyttan commented Mar 5, 2024

@gfwilliams This would probably be over engineering it. But we could make ongoing transmission-intervals of recorder logs continue after a reset with the help of .on('kill', ...).

What do you think?

@gfwilliams
Copy link
Member

we could make ongoing transmission-intervals of recorder logs continue after a reset with the help of .on('kill', ...).

Yes, I think that's a step too far... Gadgetbridge will have to detect when it's missing data anyway, so we might as well just use that functionality, rather than attempting to do the same thing twice.

In many cases users will use fast load capable clocks and messages, so transmission will continue anyway as long as they don't load an app - so I feel like there's no point complicating things for the reasonably rare case.

apps/android/boot.js Outdated Show resolved Hide resolved
@thyttan thyttan force-pushed the android-gb-activity-tracks branch 5 times, most recently from 5f411b9 to e202f66 Compare March 9, 2024 01:38
@thyttan thyttan marked this pull request as ready for review March 9, 2024 03:41
@thyttan
Copy link
Collaborator Author

thyttan commented Mar 9, 2024

@gfwilliams or @bobrippling I feel like this could possibly be done now - so please take a closer look when you get the time! :)

(I want to squash commits before pulling it)

@digitalcircuit would you be able to test this? :)

The modified Android Integraion app can be installed from my app loader: https://thyttan.github.io/BangleApps/?q=android

Here's the Gadgetbridge PR if you'd want to build from source: https://codeberg.org/Freeyourgadget/Gadgetbridge/pulls/3153

@bobrippling
Copy link
Collaborator

LGTM although I'm not very familiar with the GB stuff, @gfwilliams are you ok to merge?

(thanks for the links even so)

@gfwilliams
Copy link
Member

Yes, looks great to me! Adding extra handlers won't break anything either way :)

@thyttan
Copy link
Collaborator Author

thyttan commented Mar 11, 2024

I just fixup'ed and renamed commits. Ready for merge.

@thyttan
Copy link
Collaborator Author

thyttan commented Mar 11, 2024

...after I bumped app version...

Lets Gadgetbridge fetch logs created with the `recorder` app.
@thyttan
Copy link
Collaborator Author

thyttan commented Mar 11, 2024

There - version bumped. I should add to README as well as to https://github.com/espruino/EspruinoDocs/blob/master/info/Gadgetbridge.md .

@thyttan
Copy link
Collaborator Author

thyttan commented Mar 11, 2024

Looking at the README I don't think this needs mentioning there. So - ready for merge.

@gfwilliams
Copy link
Member

Thanks! Yes, looks good to me! Please could you do a quick PR for https://github.com/espruino/EspruinoDocs/blob/master/info/Gadgetbridge.md to mention the new JSON types there?

@gfwilliams gfwilliams merged commit 43728e0 into espruino:master Mar 11, 2024
1 check passed
@gfwilliams
Copy link
Member

wow - before I'd even hit 'comment' by the look of it - thanks!

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

3 participants