Activity notifications #1675
Activity notifications #1675
Conversation
|
Build size and comparison to main:
|
src/systemtask/SystemTask.cpp
Outdated
| if (activityController.UpdateSteps(motionController.NbSteps()) && | ||
| settingsController.GetActivity() == Controllers::Settings::Activity::On) { | ||
| NRF_LOG_INFO("activity"); | ||
| Controllers::NotificationManager::Notification notif; | ||
| constexpr char message[] = "Low activity\0Fewer steps than threshold in last hour. Try walking around."; | ||
| constexpr uint8_t messageSize = | ||
| std::min(sizeof message / sizeof(char), Pinetime::Controllers::NotificationManager::MaximumMessageSize()); | ||
| std::memcpy(notif.message.data(), message, messageSize * sizeof(char)); | ||
| notif.size = messageSize; | ||
| notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert; | ||
| notificationManager.Push(std::move(notif)); | ||
| PushMessage(Messages::OnNewNotification); | ||
| } | ||
|
|
There was a problem hiding this comment.
This should be moved to at least one function. Also I'm wondering if this should be part of DisplayApp. What DisplayApp and SystemTask should do is not defined at all, but considering that DisplayAppRecovery does not benefit from this, it probably should not be part of the "system", but rather the "application". I'm working on fixing the ambiguity.
There was a problem hiding this comment.
I agree that this should be moved to a function, but as you say, I'm not sure whether that function should be in NotificationManager, DisplayApp or even SystemTask. Because of this I won't move this to a function yet.
#1482 makes constructing a notification a lot simpler as well, so when that gets cleaned up and merged it might not even need to be moved to its own function.
FintasticMan
force-pushed
the
activity_notifications
branch
3 times, most recently
from
c3c15b1
to
fbefb52
Compare
FintasticMan
force-pushed
the
activity_notifications
branch
2 times, most recently
from
1f9d941
to
8458383
Compare
FintasticMan
force-pushed
the
activity_notifications
branch
from
8458383
to
e355baa
Compare
|
Actually I was mistaken. This is not quite like a circular buffer. A class that tracks the changes made to a variable, with template parameters specifying the resolution and how much history is stored would be more appropriate for this use case. You would access the value an hour ago through a function that you pass a time point or duration to. |
FintasticMan
force-pushed
the
activity_notifications
branch
from
e355baa
to
ac37822
Compare
FintasticMan
force-pushed
the
activity_notifications
branch
from
ac37822
to
4642dbd
Compare
FintasticMan
force-pushed
the
activity_notifications
branch
from
4642dbd
to
b262001
Compare
FintasticMan
added
new feature
Depends on #1696.
This adds a feature that sends the user a notification if they haven't walked a certain number of steps within the past hour. It works by keeping track of the number of steps every half hour, storing them in a circular buffer. It checks whether difference between the number of steps from the last half hour and the number of steps from an hour before that is lower than the threshold.
The majority of the size increase comes from the settings screen, so if I can make that smaller, that would be good.
I'm not sure if a notification is the best way to handle alerting the user. Suggestions welcome.