InfiniTime
InfiniTime copied to clipboard
Vibration scheduler
This PR aims to solve #1223 . It implements intermediate logic between the motor controller and the rest of the system. The main goal is to prevent simutaneously activation of vibrations. The side effects of the old behaviour seem to be weird strong vibrations and vibrations cancelling themselves out.
This implementations exposes the following API on the MotorController
:
ActivatePhoneCall
, ActivateTimer
, ActivateAlarm
, ActivateNotification
to mark the different vibrations as something that has to happen.
DeactivatePhoneCall
, DeactivateTimer
, DeactivateAlarm
to mark the vibrations as being over.
SingleVibration
to trigger a vibration for in-app use (eg. metronome). The single vibration is skipped, when another vibration is already happening.
The activated vibrations are handled according to their priority. The priorities from high to low are:
- phone call
- timer
- alarm
- notification
The active one with the highest priority is run. When it is over, the next highest one is run afterwards. This ensures that none of them is lost.
In the Update
function, we can also define custom durations/pattern for each of the vibrations, such that they can be easier distinguished.
TODO:
- [ ] Are chimes as single vibration ok?
- [ ] Test this code.
I thought about something like this too for a little while, but then went for the simple implementation, because what are we gaining from the abstract one?
In the long run, I also wanted to add that when a vibration starts, the corresponding app is opened.
Based on your idea, I could see some kind of initial registration from an app where it submits a config of something like
- it's name
- priority
- duration
- recurrence
And then we (de)activate based on the name.
How do we store them? Making an array of n priorities? Or one with n entries and give out ids like you did? This leads to cleaner code for the future, but uses more RAM. I like the cleaner API though.
I think the MotorController should not handle general notifications. If someone wants to implement a visual indication for notifications in the future for example this would all need to be copied or moved somewhere else.
I think it might make sense to put the handler for this into a separate Notification Controller. It might seem like bloat but I think it might get more important in the future since notifications are one of the main components of this watch.
This is also something that would enable this:
In the long run, I also wanted to add that when a vibration starts, the corresponding app is opened.
Thanks for the feedback. I will do this.
I am stuck with getting
/sources/src/systemtask/SystemTask.cpp: In constructor 'Pinetime::System::SystemTask::SystemTask(Pinetime::Drivers::SpiMaster&, Pinetime::Drivers::St7789&, Pinetime::Drivers::SpiNorFlash&, Pinetime::Drivers::TwiMaster&, Pinetime::Drivers::Cst816S&, Pinetime::Components::LittleVgl&, Pinetime::Controllers::Battery&, Pinetime::Controllers::Ble&, Pinetime::Controllers::DateTime&, Pinetime::Controllers::TimerController&, Pinetime::Controllers::AlarmController&, Pinetime::Drivers::Watchdog&, Pinetime::Controllers::NotificationManager&, Pinetime::Controllers::MotorController&, Pinetime::Drivers::Hrs3300&, Pinetime::Controllers::MotionController&, Pinetime::Drivers::Bma421&, Pinetime::Controllers::Settings&, Pinetime::Controllers::HeartRateController&, Pinetime::Applications::DisplayApp&, Pinetime::Applications::HeartRateTask&, Pinetime::Controllers::FS&, Pinetime::Controllers::TouchHandler&, Pinetime::Controllers::ButtonHandler&)':
/sources/src/systemtask/SystemTask.cpp:84:21: error: cannot bind non-const lvalue reference of type Pinetime::Controllers::AlertController&' to an rvalue of type 'Pinetime::Controllers::AlertControlle '
84 | alertController(motorController),
| ^~~~~~~~~~~~~~~
In file included from /sources/src/systemtask/SystemTask.h:21,
from /sources/src/systemtask/SystemTask.cpp:1:
/sources/src/components/alert/AlertController.h:10:7: note: after user-defined conversion: 'Pinetime::Controllers::AlertController::AlertController(Pinetime::Controllers::MotorController&)'
10 | AlertController(MotorController& motorController);
| ^~~~~~~~~~~~~~~
and can not figure out how to do this right. I would apprechiate some help with this. All I want to do is adding the AlertController to the SystemTask.
@minacode I got it to compile although I haven't tested it yet. But you can take some inspiration from what I did if you want.
https://github.com/Joheyy/InfiniTime/commit/73b12c649504c8b8e1bed7619b3d29e65e341959
Thank you very much! You can make a PR at my fork and I will merge your changes to this PR.
I am still a little confused why your approach compiles though. For example your change in SystemTask.cpp
seems to break the pattern of the other lines around.
Edit: I learned about C++ variable initialization. Was not expecting this... But things make more sense now.
Ok, so the last commit does a few things we surely will discuss.
-
It moves the call of
Update
to the SystemTask. This ensures that all active alerts will be run in the case thatUpdate
will do nothing because a single vibration from an app is running. There is still the possibility of an app spamming single vibrations or doing a very long one, but this wont happen in practice. We could add aStopAll
function to the MotorController like @ght suggested. This should then only be called by the AlertController to gain priority. -
It implements support for the DisplayApp to restore the original functionality (apps opening when vibrations start).
Update
returns whether it started a new vibration. If it did, the SystemTask gets the corresponding Message fromDisplayMessage
and then forwards it to the DisplayApp. This part seems to me like we might refactor this. One idea is to only ping the DisplayApp and letting it catch the value by itself (although that would only be more complicated). Right now, the reference to the AlertController in the DisplayApp is not needed.
Yes, that issue was the start of this work 😊
The missing part is to test it. The functionality is complete, I think. Sorry that I haven't done it yet. My focus went to other PRs.
Also I have to rebase it. This PR is behind by 100+ commits.
This is my focus PR once I have time again in the next days. If you want to help you can take a look at the code or test it. It's behind develop but it seems to run stably. We need to find out if it works as intended.
There is a minor problem with this PR: when more than one notification happens, the notification app triggers only once and will probably show only the last one. If more than five notifications happen, the first one will be deleted before it could be read by the user. What do you think about it?
@minacode I have gadgetbridge set to only allow a notification every 5 seconds anyways.