Skip to content

Commit 5c6b08b

Browse files
patgruberpatgruber
committed
implement suggestions from code review
1 parent 1d240a4 commit 5c6b08b

File tree

3 files changed

+72
-24
lines changed

3 files changed

+72
-24
lines changed

‎src/displayapp/screens/settings/SettingHeartRate.cpp‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,19 @@ namespace {
1515
}
1616
}
1717

18-
constexpr std::array<Option, 8> SettingHeartRate::options;
19-
2018
SettingHeartRate::SettingHeartRate(Pinetime::Controllers::Settings& settingsController) : settingsController {settingsController} {
2119

22-
lv_obj_t* container1 = lv_cont_create(lv_scr_act(), nullptr);
20+
lv_obj_t* container = lv_cont_create(lv_scr_act(), nullptr);
2321

24-
lv_obj_set_style_local_bg_opa(container1, LV_CONT_PART_MAIN, LV_STATE_DEFAULT, LV_OPA_TRANSP);
25-
lv_obj_set_style_local_pad_all(container1, LV_CONT_PART_MAIN, LV_STATE_DEFAULT, 5);
26-
lv_obj_set_style_local_pad_inner(container1, LV_CONT_PART_MAIN, LV_STATE_DEFAULT, 5);
27-
lv_obj_set_style_local_border_width(container1, LV_CONT_PART_MAIN, LV_STATE_DEFAULT, 0);
22+
lv_obj_set_style_local_bg_opa(container, LV_CONT_PART_MAIN, LV_STATE_DEFAULT, LV_OPA_TRANSP);
23+
lv_obj_set_style_local_pad_all(container, LV_CONT_PART_MAIN, LV_STATE_DEFAULT, 5);
24+
lv_obj_set_style_local_pad_inner(container, LV_CONT_PART_MAIN, LV_STATE_DEFAULT, 5);
25+
lv_obj_set_style_local_border_width(container, LV_CONT_PART_MAIN, LV_STATE_DEFAULT, 0);
2826

29-
lv_obj_set_pos(container1, 10, 60);
30-
lv_obj_set_width(container1, LV_HOR_RES - 20);
31-
lv_obj_set_height(container1, LV_VER_RES - 50);
32-
lv_cont_set_layout(container1, LV_LAYOUT_PRETTY_TOP);
27+
lv_obj_set_pos(container, 10, 60);
28+
lv_obj_set_width(container, LV_HOR_RES - 20);
29+
lv_obj_set_height(container, LV_VER_RES - 50);
30+
lv_cont_set_layout(container, LV_LAYOUT_PRETTY_TOP);
3331

3432
lv_obj_t* title = lv_label_create(lv_scr_act(), nullptr);
3533
lv_label_set_text_static(title, "Backg. Interval");
@@ -47,7 +45,7 @@ SettingHeartRate::SettingHeartRate(Pinetime::Controllers::Settings& settingsCont
4745
unsigned int currentInterval = settingsController.GetHeartRateBackgroundMeasurementInterval();
4846

4947
for (unsigned int i = 0; i < options.size(); i++) {
50-
cbOption[i] = lv_checkbox_create(container1, nullptr);
48+
cbOption[i] = lv_checkbox_create(container, nullptr);
5149
lv_checkbox_set_text(cbOption[i], options[i].name);
5250
cbOption[i]->user_data = this;
5351
lv_obj_set_event_cb(cbOption[i], event_handler);

‎src/heartratetask/HeartRateTask.cpp‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -213,23 +213,22 @@ void HeartRateTask::HandleSensorData(int* lastBpm) {
213213
// !IsContinuousModeActivated()
214214

215215
if (ShouldStartBackgroundMeasuring()) {
216-
// This doesn't change the state but resets the measurment timer, which basically starts the next measurment without resetting the
217-
// sensor. This is basically a fall back to continuous mode, when measurments take too long.
216+
// This doesn't change the state but resets the measurement timer, which basically starts the next measurement without resetting the
217+
// sensor. This is basically a fall back to continuous mode, when measurements take too long.
218218
measurementStart = xTaskGetTickCount();
219219
return;
220220
}
221221

222-
bool noDataWithinTimeLimit = bpm == 0 && ShoudStopTryingToGetData();
223-
bool dataWithinTimeLimit = bpm != 0;
224-
if (dataWithinTimeLimit || noDataWithinTimeLimit) {
222+
bool dataTimeout = bpm == 0 && ShouldStopTryingToGetData();
223+
bool foundHeartrate = bpm != 0;
224+
if (foundHeartrate || dataTimeout) {
225225
state = States::ScreenOffAndWaiting;
226226
StopMeasurement();
227227
}
228228
}
229229

230230
TickType_t HeartRateTask::GetBackgroundIntervalInTicks() {
231-
int ms = settings.GetHeartRateBackgroundMeasurementInterval() * 1000;
232-
return pdMS_TO_TICKS(ms);
231+
return settings.GetHeartRateBackgroundMeasurementInterval() * configTICK_RATE_HZ;
233232
}
234233

235234
bool HeartRateTask::IsContinuousModeActivated() {
@@ -244,10 +243,10 @@ TickType_t HeartRateTask::GetTicksSinceLastMeasurementStarted() {
244243
return xTaskGetTickCount() - measurementStart;
245244
}
246245

247-
bool HeartRateTask::ShoudStopTryingToGetData() {
248-
return GetTicksSinceLastMeasurementStarted() >= DURATION_UNTIL_BACKGROUND_MEASUREMENT_IS_STOPPED;
246+
bool HeartRateTask::ShouldStopTryingToGetData() {
247+
return GetTicksSinceLastMeasurementStarted() >= durationUntilBackgroundMeasurementIsStopped;
249248
}
250249

251250
bool HeartRateTask::ShouldStartBackgroundMeasuring() {
252251
return GetTicksSinceLastMeasurementStarted() >= GetBackgroundIntervalInTicks();
253-
}
252+
}

‎src/heartratetask/HeartRateTask.h‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,57 @@
55
#include <components/heartrate/Ppg.h>
66
#include "components/settings/Settings.h"
77

8-
#define DURATION_UNTIL_BACKGROUND_MEASUREMENT_IS_STOPPED pdMS_TO_TICKS(30 * 1000)
8+
/*
9+
*** Background Measurement deactivated ***
10+
11+
12+
13+
┌─────────────────────────┐ ┌─────────────────────────┐
14+
│ ├───StartMeasurement───►│ │
15+
│ ScreenOnAndStopped │ │ ScreenOnAndMeasuring │
16+
│ │◄──StopMeasurement │ │
17+
└──▲────────────────┬─────┘ └──▲──────────────────┬───┘
18+
│ │ │ │
19+
WakeUp GoToSleep WakeUp GoToSleep
20+
│ │ │ │
21+
┌──┴────────────────▼─────┐ ┌──┴──────────────────▼───┐
22+
│ │ │ │
23+
│ ScreenOffAndStopped │ │ ScreenOffAndWaiting │
24+
│ │ │ │
25+
└─────────────────────────┘ └─────────────────────────┘
26+
27+
28+
29+
30+
31+
*** Background Measurement activated ***
32+
33+
34+
35+
┌─────────────────────────┐ ┌─────────────────────────┐
36+
│ ├───StartMeasurement───►│ │
37+
│ ScreenOnAndStopped │ │ ScreenOnAndMeasuring │
38+
│ │◄──StopMeasurement │ │
39+
└──▲────────────────┬─────┘ └──▲──────────────────┬───┘
40+
│ │ ┌───────┘ │
41+
WakeUp GoToSleep │ WakeUp GoToSleep
42+
│ │ │ │ │
43+
┌──┴────────────────▼─────┐ │ ┌──┴──────────────────▼───┐
44+
│ │ │ │ │
45+
│ ScreenOffAndStopped │ │ │ ScreenOffAndWating │
46+
│ │ │ │ │
47+
└─────────────────────────┘ │ └───┬──────────────────▲──┘
48+
│ │ │
49+
│ Waited Got sensor
50+
│ interval data
51+
│ time │
52+
│ │ │
53+
WakeUp ┌───▼──────────────────┴──┐
54+
│ │ │
55+
└────┤ ScreenOffAndMeasuring │
56+
│ │
57+
└─────────────────────────┘
58+
*/
959

1060
/*
1161
*** Background Measurement deactivated ***
@@ -81,6 +131,7 @@ namespace Pinetime {
81131
void Start();
82132
void Work();
83133
void PushMessage(Messages msg);
134+
static constexpr int durationUntilBackgroundMeasurementIsStopped = pdMS_TO_TICKS(30 * 1000);
84135

85136
private:
86137
static void Process(void* instance);
@@ -100,7 +151,7 @@ namespace Pinetime {
100151
bool IsBackgroundMeasurementActivated();
101152

102153
TickType_t GetTicksSinceLastMeasurementStarted();
103-
bool ShoudStopTryingToGetData();
154+
bool ShouldStopTryingToGetData();
104155
bool ShouldStartBackgroundMeasuring();
105156

106157
TaskHandle_t taskHandle;

0 commit comments

Comments
 (0)