Correct OTA process message show on display #137
Conversation
| if (ag->isOne()) { | ||
| oledDisplay.showNewFirmwareSkipped(); | ||
| } else { | ||
| Serial.println("Firmware update: Skipped"); |
There was a problem hiding this comment.
Propose to rephrase to
Firmware update skipped
| if (ag->isOne()) { | ||
| oledDisplay.showNewFirmwareUpToDate(); | ||
| } else { | ||
| Serial.println("Firmware update: up to date"); |
There was a problem hiding this comment.
Propose to rephrase to
Firmware is up to date
examples/OneOpenAir/OneOpenAir.ino
Outdated
| @@ -529,6 +549,11 @@ static void displayExecuteOta(OtaState state, String msg, int processing) { | |||
| break; | |||
| } | |||
| case OtaState::OTA_STATE_SUCCESS: { | |||
| if (ag->isOne()) { | |||
| oledDisplay.showNewFirmwareUpdating(String(100)); | |||
There was a problem hiding this comment.
Not related to the PR but I would expect that we can write this here in this context
oledDisplay.showFirmwareUpdateProgress(100);
There was a problem hiding this comment.
It's relate to PR. Because firmware process has finish 100% but display sometime do not show 100%.
There was a problem hiding this comment.
Ehm, I mean that we should pass the value along as an int (and not a String)
| @@ -544,6 +569,7 @@ static void displayExecuteOta(OtaState state, String msg, int processing) { | |||
|
|
|||
| delay(1000); | |||
| } | |||
| oledDisplay.setBrightness(0); | |||
There was a problem hiding this comment.
Curious to know, what happens without this line?
There was a problem hiding this comment.
This like will be clear all display content while reboot device. If not the display still Show "Firmware update success" while rebooting
| @@ -857,11 +883,12 @@ static void configUpdateHandle() { | |||
| fwNewVersion = configuration.newFirmwareVersion(); | |||
| if (fwNewVersion.length()) { | |||
| bool doOta = false; | |||
| if (measurements.otaBootCount == 0) { | |||
| if (measurements.otaBootCount < 0) { | |||
| doOta = true; | |||
| Serial.println("First OTA"); | |||
There was a problem hiding this comment.
I think the message is misleading, the device might have gone through several OTA updates when we arrive here.
There was a problem hiding this comment.
It's indicate first OTA check. If it's zero and process failed, retry will call for 3-4 times (Because we check cloud configure each 15sec). < 0 should help, that mean the otaBootCount is invalid, after first check it's set to bootCount
| DISP()->firstPage(); | ||
| do { | ||
| DISP()->setFont(u8g2_font_t0_16_tf); | ||
| setCentralText(20, "Firmware Update"); |
There was a problem hiding this comment.
Propose to rephrase to
Firmware update skipped
There was a problem hiding this comment.
If it's fit for a line we should reduce the font size.
src/AgOledDisplay.h
Outdated
| @@ -40,6 +40,8 @@ class OledDisplay : public PrintLog { | |||
| void showNewFirmwareUpdating(String percent); | |||
There was a problem hiding this comment.
propose to rename to
showFormwareUpdateProgress(int percent)
| DISP()->firstPage(); | ||
| do { | ||
| DISP()->setFont(u8g2_font_t0_16_tf); | ||
| setCentralText(20, "Firmware Update"); |
There was a problem hiding this comment.
The term Update is redundant this way, propose to make it a sentence and say
Firmware
is up to date
examples/OneOpenAir/OneOpenAir.ino
Outdated
| @@ -529,6 +549,11 @@ static void displayExecuteOta(OtaState state, String msg, int processing) { | |||
| break; | |||
| } | |||
| case OtaState::OTA_STATE_SUCCESS: { | |||
| if (ag->isOne()) { | |||
| oledDisplay.showNewFirmwareUpdating(String(100)); | |||
There was a problem hiding this comment.
Ehm, I mean that we should pass the value along as an int (and not a String)
No description provided.