as I really want to get HR Data into runnerup here is a patch to do so. It is still not finished however I am proposing at an early stage to get some feedback. The Idea is to provide three query apis to:
Discover Device(s?) The Interface looks like you can pair multiple devices. Hence an endpoint to discover them. As I only have a MiBand2 I am not 100% sure how Gadgetbridge behaves here
Start / Stop Realtime DataCollection: It seems like I have to send an Intent to start that
Get Realtime Data: Send Samples and inform ContentObservers
I have written some tests. What works for sure is discovery of a device. Realtime data is currently delivered as String, that is also a TODO. Furthermore my activity_start / stop are a stub in the tests.
However before implementing Contract Classes, deciding on the data format, writing more tests etc. I would like to have some feedback, if the API meets your "taste", what Data you would send (only Heartrate, Heartrate and Steps, Full Sample Object). Furthermore some input on the Code, how to test everything is highly appreciated. If you think ContentProvider is not the way to go, that is also fine.
About the Travis error: The testsuite does not set up the DeviceManager and I have no idea how to do that. Strangely locally all tests pass.
So I addressed your comments for the better part. What is left now to do is:
After some measurements I do not get any more heartrates from my Mi2BandSupport. This is unfortunate. It may have several reasons including: A bug in my code, a cursed firmware, a thread inside of GB which just shuts that reporting down. Any thoughts / pointers here? Could someone with another device test? Do I need to take a Wakelock somewhere?
Fix Travis: Tests fail because I try to query the device Manager which is not set up correctly (at least getDeviceManager() in GBApplication returns null) in the test environment. I am open to suggestions for workarounds.
Permission System to read HR Data: I think android asking for permission instead of just send HR Date to every interested party would be a nice thing to do
I found a Workaround for the missing samples problems. In LiveActivityFragment there is this:
which repeatedly calls that function. So I also added a Timer, that calls onEnableRealtimeHeartRateMeasurement over and over again. Though I think this is strange behaviour, as the realtimesteps keep coming without any periodic enabling... Anyway, everything works now.
I also added the permissions, hence I finished my todo list and will commit the changes to this branch.
GBApplication is not set up correctly in onCreate(). It checks wether a static variable is set already and if so it returns. Rationale is to guard against multiple invocations from robolectric. However the DeviceManager is not static, therefore each invocation of the testsuite with more than one testcase will get null as a result for ((GBApplication)getContext()).getDeviceManager() in the second and following testcases.
@cpfeiffer actually it was you, who committed that change two years ago:
It simply calls getDeviceManager() in two testcases. The first works, the second fails.
re timer: yes, we have to re-enable the realtime measurements again and again for Mi2. It is also kind of sensible, because delivering realtime data is much more battery intensive than sending the data in one go. So Mi2 only sends it for a short period of time if it doesn't get another enable-command.
re DeviceManager: back then we got multiple invocations of GBApplication#onCreate() by robolectric, either due to a bug or by design. Things may have changed since then, so we might revise this. I totally agree that this code is a little convoluted, with the db being setup either by GBApplication (in normal mode) or by TestBase (in test mode). Initially, robolectric did not support custom Application subclasses at all and I don't know how it handles the Application instance lifecycle at the moment. For db tests, we want to make sure that every test gets a fresh db environment.
According to your findings, I suspect that we do get a new GBApplication instance (hence deviceManager field being null). The static fields survive, of course.
So to sum up,
we can check if we still get multiple onCreate() calls on the same instance
we can try to make all static instances in GBApplication non-static and then remove the guard
or we could make the deviceManager instance also static (urgh)
or come up with something else?
I think we can have realtime measurements without sending commands over and over again. Not 100% sure though
Thanks for your work, really looking forward to getting this in! 👍
About the tests: I did not see multiple invocations of onCreate during my (limited) tests. I saw one invocation per testcase. Hence from that point of view I should be o.k. to remove the guard.
Add a library via maven which contains the ContractClass for easier integration
Change the ContentProvider from this quite rudimentary buffered_sample in Memory thing to use the database as every realtime measurement automatically lands there too
Provide historic data
After removing the guard all 61 tests still pass. onCreate is called 61 times.
And to answer that question: Yes this starts gadgetbridge in case it was not running before.
Sound lovely, I'll address the comment and squash it to a bare minimum. I'll ping you when I am done. In the meantime, might I motivate you to compile my runnerup fork and try it out with different / multiple devices? You don't need to go running, just some 5 mins of testing in your pocket would be fantastic.
Furthermore, for the record: as soon as this change is merged, you have an external api, which others will develop against. Hence I'd flag it as "beta" or "unstable" in the changelog entry for the first version.
I have one suggestion though, this content provider should be protected by an authorization dialogue before being released to the public and doing it right now while nothing depends on it sounds like a reasonable idea:
Fragment for managing content provider accesses
Database or shared preferences model for storing package names and granted access(es, if we add more providers later on)
Content provider has to check calling package (For ex. ContentProvider.getCallingPackage)
Hi @TaaviE, thx for your comment. I think your wish is actually an OS function based on the android permissions system (see screenshot, the default is off, which I changed). Together with the suggestion by cpfeiffer (the big todo) it is guaranteed that the user first has to give his consent an then also sees an indicator that realtime monitoring is happening. And this will be the topic of my next comment :)
Hi @cpfeiffer, I like the idea of showing a notification to the user and allowing her to interrupt realtime sampling. However that is actually not as easy to do, because there is a thread inside the contentprovider (and LiveActivityFragment but that should not be the problem), that restarts that sampling each second.
@boun Gadgetbridge's minimum API is Kitkat (19, 4.4) and runtime permissions are a Marshmallow (23+) thing meaning that this has to be implemented by Gadgetbridge.
@TaaviE It is not about runtime permissions. Here the screenshot from KitKat. I do not think it makes sense to rebuild a OS function, that is available in later releases. The Users is informed upon install that the app wants to access these services. And if the "big todo" is implemented she is informed in realtime and might even stop it from happening.
However this is my opinion, if you want to implement that feel free!
@boun Exactly my point, everyone who wants gets access when there's just that permission on KK. Say some proprietary app actually adds Gadgetbridge support I and I suspect many others would not want it to instantly have access to my heart rate data.
I consider your point highly hypothetical :) However feel free to implement it.
Given the nature of the project, I agree that the feature outlined by @TaaviE is important and should be present when the new functionality is available in the published application.
Which means (in my opinion) that it could either be implemented in the PR, or after the merge (but before we release).
I'm in a middle position. IMHO it shouldn't be a blocker for the first release. AFAIU, this is only a real problem for KK, where you cannot deny permissions. On Lollipop and above, a user could install a "rogue" app and simple deny the single permission.
A simple solution might be to have this feature on Lollipop+ only. Not sure if that would be a problem for the RunnerUp userbase. but I could imagine they wouldn't want to require a Gadgetbridge permission anyway, if they still targeted KK.
I personally had not the possibility to try out the integration yet, maybe next week.
What's the current state of this? Can we move this forward?
Since I'm very interested in being able to track my HR in an opensource running app, I tried this with my miband3. I probably did something wrong, because GB loses the connection to my mb3 after about 5 minutes and crashes when I try to export the logs from the debug screen. @boun if you're still interested in working on this, I can help test it but I'm no code wizard so I won't be able to do much by myself except run ADB and find out where it fails. I have runnerup from your contentprovider branch crashing as well when trying to get into the history pane, but I don't think this has anything to do with your changes.
Anyway, if I can help with something that's not way over my head, please let me know, I'd be happy to.
@lesensei Is this reproduceable? Can you provide logs (privately, if you want)?