InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.71k stars 926 forks source link

Recovery doesn't build because DisplayAppRecovery::Start() doesn't take new BootError #742

Closed Quantum-cross closed 2 years ago

Quantum-cross commented 2 years ago

can't compile your branch locally getting error

[build] /home/nero/repos/pinetime/InfiniTime/src/systemtask/SystemTask.cpp: In member function 'void Pinetime::System::SystemTask::Work()':
[build] /home/nero/repos/pinetime/InfiniTime/src/systemtask/SystemTask.cpp:159:29: error: no matching function for call to 'Pinetime::Applications::DisplayApp::Start(Pinetime::System::BootErrors&)'
[build]   159 |   displayApp.Start(bootError);
[build]       |                             ^
[build] In file included from /home/nero/repos/pinetime/InfiniTime/src/systemtask/SystemTask.h:25,
[build]                  from /home/nero/repos/pinetime/InfiniTime/src/systemtask/SystemTask.cpp:1:
[build] /home/nero/repos/pinetime/InfiniTime/src/displayapp/DisplayAppRecovery.h:60:12: note: candidate: 'void Pinetime::Applications::DisplayApp::Start()'
[build]    60 |       void Start();
[build]       |            ^~~~~
[build] /home/nero/repos/pinetime/InfiniTime/src/displayapp/DisplayAppRecovery.h:60:12: note:   candidate expects 0 arguments, 1 provided
[build] make[2]: *** [src/CMakeFiles/pinetime-mcuboot-recovery.dir/build.make:664: src/CMakeFiles/pinetime-mcuboot-recovery.dir/systemtask/SystemTask.cpp.o] Error 1

do I maybe need to update a dependency (other than the lvgl submodule)?

Originally posted by @NeroBurner in https://github.com/InfiniTimeOrg/InfiniTime/issues/734#issuecomment-940433456

Quantum-cross commented 2 years ago

I fixed this locally in #734 by overloading Start()

void Start(Pinetime::System::BootErrors){ Start(); };

If this is acceptable I can PR it.

geekbozu commented 2 years ago

I think a question to ask here is the recovery even getting an update anytime soon? It should definitely be fixed get some of the other updates/TWI fixes but what is the expected update cycle on it? Is there one?

JF002 commented 2 years ago

I was wondering why the Actions did not fail when I merged those changes, but it looks like it doesn't build the recovery targets... That exactly why I would like to use Docker in the CI instead of the .yaml files from Github. See https://github.com/InfiniTimeOrg/InfiniTime/issues/674. @Quantum-cross's suggestion is probably a valid simple fix :) @geekbozu I do not test the recovery firmware for each release because it takes a lot of time to ensure it does its job well (as it is the last chance before a brick). That's also why I don't release it for each release of InfiniTime :) Now, you're right : this recovery firmware totally deserves some love and should be maintained to take advantages of improvements from InfiniTime! So yeah, once we fix these issues and test it thoroughly, we will be able to release a new version of the recovery!tran

NeroBurner commented 2 years ago

I think this issue is solved and can be closed, isn't it?

geekbozu commented 2 years ago

Yes it can :)