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

feat: Add support for AdamHealth Sensor #617

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

geekytwink
Copy link

Tested with Intiface Central's Read Sensor button. This exercises the event stream subscription code path, but I'm not sure how to test the full subscription experience.
I look forward to your feedback! <3

image

@CLAassistant
Copy link

CLAassistant commented Mar 12, 2024

CLA assistant check
All committers have signed the CLA.

@qdot
Copy link
Member

qdot commented Mar 16, 2024

We... don't actually have a good way to test subscriptions yet. That's a good callout, I should add something for that to Intiface.

I'm about to do a release of Buttplug but I'm gonna hold off on bringing this PR in until the next release, 'cause I need to get an IC release out now and there's a couple of things I need to think about in terms of having to hold a subscription to get things like battery. While on its face I think it's ok, that means if someone reads battery info, we start the subscription then it's just going, which could lead to power draw issues. I doubt we'd kill phones this way, it's more of an interesting thing to think about.

Just curious, what's the update rate of the sensor?

@geekytwink
Copy link
Author

No rush to merge, thank you for taking a look!
I think the intention of the sender.receiver_count check (which I copied from the kiiroo_v21 module) is to unsubscribe if all event_streams get dropped, but I may be assuming incorrectly. I was also struggling a bit with calling handle_sensor_unsubscribe_cmd and wanted to get the code out for an initial review first :)

Trying more on this, turns out receiver_count will always have at least 1 for the sender, that's an easy fix. However, the spawned coroutine doesn't have access to &self, and trying to pull the unsubscription logic to a helper function couldn't figure out the lifetime for the BoxFuture. For now I've duplicated the unsubscribe code, but I want to know how to factor it out.

How does receiver_count normally work? How would the sender get dropped to reduce the count to 0, since it's moved into that loop?
This receiver_count trick does mean that any clients must manually load an event_stream before the subscribe call. This is where I'm not sure I've understood the architecture right, so let me know if I'm going down the right path!

The btle subscription effectively updates the sensor values roughly once a second:

flutter: 🐛 19:04:43.627445 DEBUG    Global Loggy - "1300"
flutter: 🐛 19:04:43.867139 DEBUG    Global Loggy - "0"
flutter: 🐛 19:04:44.107485 DEBUG    Global Loggy - "1301"
flutter: 🐛 19:04:44.347294 DEBUG    Global Loggy - "59"
flutter: 🐛 19:04:44.587371 DEBUG    Global Loggy - "1300"
flutter: 🐛 19:04:44.827170 DEBUG    Global Loggy - "0"
flutter: 🐛 19:04:45.067102 DEBUG    Global Loggy - "1301"
flutter: 🐛 19:04:45.307324 DEBUG    Global Loggy - "60"
flutter: 🐛 19:04:45.614219 DEBUG    Global Loggy - "1300"
flutter: 🐛 19:04:45.847080 DEBUG    Global Loggy - "0"

@qdot
Copy link
Member

qdot commented May 13, 2024

I swear I haven't forgotten about this. Gonna get v8 out this week then probably bring this in.

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