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.

Sign up for GitHub

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

Notification age (x minutes ago) on notifcation screen #835

Closed
wants to merge 4 commits into from
Closed

Notification age (x minutes ago) on notifcation screen #835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8466196
first try at adding notification age
clemensvonmolo Nov 17, 2021
1de1d44
minor changes to notification age logic
clemensvonmolo Dec 28, 2021
a7e8139
Merge branch 'develop' into notification-time
clemensvonmolo Jul 1, 2022
afe96d5
fix outdated constructor call for NotificationItem
clemensvonmolo Jul 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/components/ble/NotificationManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ constexpr uint8_t NotificationManager::MessageSize;
void NotificationManager::Push(NotificationManager::Notification&& notif) {
notif.id = GetNextId();
notif.valid = true;
notif.timeArrived = std::chrono::system_clock::to_time_t(dateTimeController.CurrentDateTime());
newNotification = true;
if (beginIdx > 0) {
--beginIdx;
Expand Down
6 changes: 6 additions & 0 deletions src/components/ble/NotificationManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#include <atomic>
#include <cstddef>
#include <cstdint>
#include <chrono>
#include "components/datetime/DateTimeController.h"

namespace Pinetime {
namespace Controllers {
Expand All @@ -30,13 +32,16 @@ namespace Pinetime {
Id id = 0;
bool valid = false;
uint8_t size;
std::time_t timeArrived;
std::array<char, MessageSize + 1> message;
Categories category = Categories::Unknown;

const char* Message() const;
const char* Title() const;
};

NotificationManager(Controllers::DateTime& dateTimeController) : dateTimeController {dateTimeController} {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can even store a const reference, to make it explicit, that a notification can't change the date-time, just get the current date and time

Suggested change
NotificationManager(Controllers::DateTime& dateTimeController) : dateTimeController {dateTimeController} {
NotificationManager(const Controllers::DateTime& dateTimeController) : dateTimeController {dateTimeController} {

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At present it is not possible since DateTime controller updates it member variables during time retrieving.

InfiniTime/src/components/datetime/DateTimeController.cpp

Lines 72 to 102 in d69cfcf

std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds> DateTime::CurrentDateTime() {
xSemaphoreTake(mutex, portMAX_DELAY);
UpdateTime(nrf_rtc_counter_get(portNRF_RTC_REG), false);
xSemaphoreGive(mutex);
return currentDateTime;
}
void DateTime::UpdateTime(uint32_t systickCounter, bool forceUpdate) {
// Handle systick counter overflow
uint32_t systickDelta = 0;
if (systickCounter < previousSystickCounter) {
systickDelta = static_cast<uint32_t>(portNRF_RTC_MAXTICKS) - previousSystickCounter;
systickDelta += systickCounter + 1;
} else {
systickDelta = systickCounter - previousSystickCounter;
}
auto correctedDelta = systickDelta / configTICK_RATE_HZ;
// If a second hasn't passed, there is nothing to do
// If the time has been changed, set forceUpdate to trigger internal state updates
if (correctedDelta == 0 && !forceUpdate) {
return;
}
auto rest = systickDelta % configTICK_RATE_HZ;
if (systickCounter >= rest) {
previousSystickCounter = systickCounter - rest;
} else {
previousSystickCounter = static_cast<uint32_t>(portNRF_RTC_MAXTICKS) - (rest - systickCounter - 1);
}
currentDateTime += std::chrono::seconds(correctedDelta);

It can be overcomed with mutable but I am not sure whether this is a good idea.

}
void Push(Notification&& notif);
Notification GetLastNotification() const;
Notification Get(Notification::Id id) const;
Expand All @@ -57,6 +62,7 @@ namespace Pinetime {
size_t NbNotifications() const;

private:
Controllers::DateTime& dateTimeController;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can even store a const reference, to make it explicit, that a notification can't change the date-time, just get the current date and time

Suggested change
Controllers::DateTime& dateTimeController;
const Controllers::DateTime& dateTimeController;

Notification::Id nextId {0};
Notification::Id GetNextId();
const Notification& At(Notification::Idx idx) const;
Expand Down
2 changes: 2 additions & 0 deletions src/displayapp/DisplayApp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ void DisplayApp::LoadApp(Apps app, DisplayApp::FullRefreshDirections direction)
notificationManager,
systemTask->nimble().alertService(),
motorController,
dateTimeController,
*systemTask,
Screens::Notifications::Modes::Normal);
ReturnApp(Apps::Clock, FullRefreshDirections::Up, TouchEvents::SwipeUp);
Expand All @@ -358,6 +359,7 @@ void DisplayApp::LoadApp(Apps app, DisplayApp::FullRefreshDirections direction)
notificationManager,
systemTask->nimble().alertService(),
motorController,
dateTimeController,
*systemTask,
Screens::Notifications::Modes::Preview);
ReturnApp(Apps::Clock, FullRefreshDirections::Up, TouchEvents::SwipeUp);
Expand Down
34 changes: 34 additions & 0 deletions src/displayapp/screens/Notifications.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ Notifications::Notifications(DisplayApp* app,
Pinetime::Controllers::NotificationManager& notificationManager,
Pinetime::Controllers::AlertNotificationService& alertNotificationService,
Pinetime::Controllers::MotorController& motorController,
Pinetime::Controllers::DateTime& dateTimeController,
System::SystemTask& systemTask,
Modes mode)
: Screen(app),
notificationManager {notificationManager},
alertNotificationService {alertNotificationService},
motorController {motorController},
dateTimeController {dateTimeController},
systemTask {systemTask},
mode {mode} {

Expand All @@ -30,6 +32,8 @@ Notifications::Notifications(DisplayApp* app,
notification.Message(),
1,
notification.category,
notification.timeArrived,
std::chrono::system_clock::to_time_t(this->dateTimeController.CurrentDateTime()),
notificationManager.NbNotifications(),
alertNotificationService,
motorController);
Expand Down Expand Up @@ -105,6 +109,8 @@ void Notifications::Refresh() {
notification.Message(),
currentIdx + 1,
notification.category,
notification.timeArrived,
std::chrono::system_clock::to_time_t(this->dateTimeController.CurrentDateTime()),
notificationManager.NbNotifications(),
alertNotificationService,
motorController);
Expand Down Expand Up @@ -186,6 +192,8 @@ bool Notifications::OnTouchEvent(Pinetime::Applications::TouchEvents event) {
previousNotification.Message(),
currentIdx + 1,
previousNotification.category,
previousNotification.timeArrived,
std::chrono::system_clock::to_time_t(dateTimeController.CurrentDateTime()),
notificationManager.NbNotifications(),
alertNotificationService,
motorController);
Expand Down Expand Up @@ -213,6 +221,8 @@ bool Notifications::OnTouchEvent(Pinetime::Applications::TouchEvents event) {
nextNotification.Message(),
currentIdx + 1,
nextNotification.category,
nextNotification.timeArrived,
std::chrono::system_clock::to_time_t(dateTimeController.CurrentDateTime()),
notificationManager.NbNotifications(),
alertNotificationService,
motorController);
Expand All @@ -237,6 +247,8 @@ Notifications::NotificationItem::NotificationItem(Pinetime::Controllers::AlertNo
0,
Controllers::NotificationManager::Categories::Unknown,
0,
0,
0,
alertNotificationService,
motorController) {
}
Expand All @@ -245,6 +257,8 @@ Notifications::NotificationItem::NotificationItem(const char* title,
const char* msg,
uint8_t notifNr,
Controllers::NotificationManager::Categories category,
std::time_t timeArrived,
std::time_t timeNow,
uint8_t notifNb,
Pinetime::Controllers::AlertNotificationService& alertNotificationService,
Pinetime::Controllers::MotorController& motorController)
Expand All @@ -270,6 +284,26 @@ Notifications::NotificationItem::NotificationItem(const char* title,
lv_obj_t* alert_count = lv_label_create(container, nullptr);
lv_label_set_text_fmt(alert_count, "%i/%i", notifNr, notifNb);
lv_obj_align(alert_count, NULL, LV_ALIGN_IN_TOP_RIGHT, 0, 16);
// almost impossible to receive a real notification at time 0, so skip because it is the "no notifications" notification
if (timeNow != 0) {
auto diff = std::chrono::system_clock::from_time_t(timeNow) - std::chrono::system_clock::from_time_t(timeArrived);
std::chrono::minutes age = std::chrono::duration_cast<std::chrono::minutes>(diff);
uint32_t ageInt = static_cast<uint32_t>(age.count());
char timeUnit;
if ((ageInt / (60 * 24)) >= 1) {
ageInt /= (60 * 24);
timeUnit = 'd';
} else if ((ageInt / 60) >= 1) {
Comment on lines +293 to +296
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be simplified.

Suggested change
if ((ageInt / (60 * 24)) >= 1) {
ageInt /= (60 * 24);
timeUnit = 'd';
} else if ((ageInt / 60) >= 1) {
if (ageInt >= 60 * 24) {
ageInt /= (60 * 24);
timeUnit = 'd';
} else if (ageInt >= 60) {

ageInt /= 60;
timeUnit = 'h';
} else {
timeUnit = 'm';
}
lv_obj_t* alert_age = lv_label_create(container, nullptr);
Copy link
Contributor

@Riksu9000 Riksu9000 Jul 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variables names should be like this. Don't commit this suggestion.

Suggested change
lv_obj_t* alert_age = lv_label_create(container, nullptr);
lv_obj_t* alertAge = lv_label_create(container, nullptr);

lv_label_set_text_fmt(alert_age, "%d%c ago", ageInt, timeUnit);
// same format as alert_count
lv_obj_align(alert_age, container, LV_ALIGN_IN_BOTTOM_RIGHT, 0, -16);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will make the padding similar to the text.

Suggested change
lv_obj_align(alert_age, container, LV_ALIGN_IN_BOTTOM_RIGHT, 0, -16);
lv_obj_align(alert_age, container, LV_ALIGN_IN_BOTTOM_RIGHT, -10, -10);

}

lv_obj_t* alert_type = lv_label_create(container, nullptr);
lv_obj_set_style_local_text_color(alert_type, LV_LABEL_PART_MAIN, LV_STATE_DEFAULT, LV_COLOR_MAKE(0xb0, 0xb0, 0xb0));
Expand Down
6 changes: 6 additions & 0 deletions src/displayapp/screens/Notifications.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
#include <FreeRTOS.h>
#include <cstdint>
#include <memory>
#include <chrono>
#include "displayapp/screens/Screen.h"
#include "components/ble/NotificationManager.h"
#include "components/datetime/DateTimeController.h"
#include "components/motor/MotorController.h"
#include "systemtask/SystemTask.h"

Expand All @@ -23,6 +25,7 @@ namespace Pinetime {
Pinetime::Controllers::NotificationManager& notificationManager,
Pinetime::Controllers::AlertNotificationService& alertNotificationService,
Pinetime::Controllers::MotorController& motorController,
Pinetime::Controllers::DateTime& dateTimeController,
System::SystemTask& systemTask,
Modes mode);
~Notifications() override;
Expand All @@ -39,6 +42,8 @@ namespace Pinetime {
const char* msg,
uint8_t notifNr,
Controllers::NotificationManager::Categories,
std::time_t timeArrived,
std::time_t timeNow,
uint8_t notifNb,
Pinetime::Controllers::AlertNotificationService& alertNotificationService,
Pinetime::Controllers::MotorController& motorController);
Expand Down Expand Up @@ -66,6 +71,7 @@ namespace Pinetime {
private:
Pinetime::Controllers::NotificationManager& notificationManager;
Pinetime::Controllers::AlertNotificationService& alertNotificationService;
Pinetime::Controllers::DateTime& dateTimeController;
Pinetime::Controllers::MotorController& motorController;
System::SystemTask& systemTask;
Modes mode = Modes::Normal;
Expand Down
2 changes: 1 addition & 1 deletion src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ Pinetime::Controllers::MotorController motorController {};
Pinetime::Controllers::DateTime dateTimeController {settingsController};
Pinetime::Drivers::Watchdog watchdog;
Pinetime::Drivers::WatchdogView watchdogView(watchdog);
Pinetime::Controllers::NotificationManager notificationManager;
Pinetime::Controllers::NotificationManager notificationManager {dateTimeController};
Pinetime::Controllers::MotionController motionController;
Pinetime::Controllers::TimerController timerController;
Pinetime::Controllers::AlarmController alarmController {dateTimeController};
Expand Down