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.

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

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

Closed
Quantum-cross opened this issue Oct 13, 2021 · 5 comments

Comments

@Quantum-cross
Copy link
Contributor

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 #734 (comment)

@Quantum-cross
Copy link
Contributor Author

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

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

If this is acceptable I can PR it.

@geekbozu
Copy link
Member

geekbozu commented Oct 13, 2021

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
Copy link
Collaborator

JF002 commented Oct 14, 2021

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 #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 added a commit to NeroBurner/InfiniTime that referenced this issue Oct 18, 2021
The recovery image "it is the last chance before a brick",
as described in InfiniTimeOrg#742 (comment)

So just build it to make sure it doesn't silently break, but don't upload it.
@NeroBurner
Copy link
Contributor

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

@geekbozu
Copy link
Member

Yes it can :)

grad0s pushed a commit to aramcon-badge/InfiniTime that referenced this issue Jun 25, 2022
The recovery image "it is the last chance before a brick",
as described in InfiniTimeOrg/InfiniTime#742 (comment)

So just build it to make sure it doesn't silently break, but don't upload it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants