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

Keep updating motion during sleep when Bluetooth is on #1207

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

Conversation

apilat
Copy link
Contributor

@apilat apilat commented Jun 29, 2022

Since connected Bluetooth devices can request motion data at any point, we should keep updating it when we are connected to one. Currently stale data will be delivered when the screen is turned off.
Some wake up settings already cause motion data to be updated with the screen off so this change should not have a significant impact on battery life.

@StarGate01
Copy link
Contributor

StarGate01 commented Jun 30, 2022

This is also enabled in #1133 .

Keep in mind that continuously broadcasting the motion data consumes quite a bit of power, refer to https://github.com/StarGate01/p8b-infinitime/tree/master/poweranalysis (The analysis was performed for my high-frequency motion implementation, but it should give a rough idea still) . Ideally, the user would have the option to toggle this broadcast.

@apilat
Copy link
Contributor Author

apilat commented Jun 30, 2022

Continuously polling the accelerometer during sleep is also enabled by some features in order to detect wakeup so I assume that the power used for this is reasonably small or at least acceptable, but if not I'm willing to make this behavior optional.
If I'm reading the code correctly, this if will prevent broadcasting unless the companion specifically requested notifications, in which case I think using some power for this is reasonable (otherwise we would be providing incorrect data).

@StarGate01
Copy link
Contributor

StarGate01 commented Jun 30, 2022

First of all, I like this change. I use apps to read the acceleration data as well.

Concerning the code flow: You are correct, the service only sends out data if the peer is subscribed. So for a default use-case, power consumption concerning the BLE radio should stay the same.

The new motion data is ingressed via MotionController::Update, which is called by SystemTask::UpdateMotion. The system task code returns early if the watch is sleeping, and does not need to wake up by a motion event. In my PR I removed the whole sleeping check , since all the function does afterwards is updating the motion service and checking the wake-up conditions again.

Your patch, if I read that correctly, adds an additional pass-through condition bleController.IsConnected, which is essentially most of the time in normal use (I guess).

Continuously polling the accelerometer during sleep is also enabled by some features in order to detect wakeup

This is only enabled if you want the watch to wake up at motion events.

To wrap up, I am debating if we need that check at all, or if it could be more-fine grained - e.g. checking if the BLE motion service is subscribed instead of checking if Bluetooth is connected in general. I think giving the user the option to conserve energy by disabling motion wakeup should be considered.

Maybe @Riksu9000 has some thoughts on this.

@Riksu9000
Copy link
Contributor

In general we should make the behaviour the most efficient and require the least input from the user. In this case the best solution to me seems to be updating motion only when it is needed, so when a motion based wakeup method is enabled or the service is subscribed. That way the user can save battery by disabling wakeup methods and the motion data is still accessible when needed.

@apilat
Copy link
Contributor Author

apilat commented Jun 30, 2022

In this case the best solution to me seems to be updating motion only when it is needed, so when a motion based wakeup method is enabled or the service is subscribed.

This is now implemented. I couldn't find an existing way to access MotionService from SystemTask so I ended up adding MotionController::GetService().

@FintasticMan
Copy link
Member

Would it not be better to add MotionService to SystemTask's constructor? That seems to be how it is done in most other cases.

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.

Motion Controller issue when screen off
4 participants