InfiniTimeOrg / InfiniTime

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

Github Simulate Action fails but Build Action succeeds #88

Closed hrmckay closed 4 years ago

hrmckay commented 4 years ago

I followed lupyuen's instructions for building the Github Actions to Simulate and Build InfiniTime. I made a change to clock.h to add a new variable "lv_obj_t* dayofweekday;" and I made changes in clock.cpp to use that variable. When the actions ran the Simulate action failed but the Build action worked. The Simulate action failed with "error: use of undeclared identifier 'dayofweekday'". It looks like the Simulate action is not using the clock.h in my repository.

Hugh

JF002 commented 4 years ago

Could you give us the link to your repo so that we can have a look at the code? Note that I'm not using Github Actions, and I might not be able to help you, but I hope someone else will be able to help you debug this issue.

hrmckay commented 4 years ago

Since lupyuen set up these actions I think he is probably the best person to answer this, but I couldn't see any way to post an issue to his documentation. I am using https://github.com/hrmckay/Pinetime

Hugh

lupyuen commented 4 years ago

Hi @hrmckay: The InfiniTime Simulator runs in a "Sandbox" and only changes to Clock.cpp are accepted. No other files may be changed. This is indicated in the doc...

Screenshot 2020-10-08 at 10 27 39 AM

A workaround is to declare dayofweekday as a static variable in Clock.cpp...

Screenshot 2020-10-08 at 10 46 05 AM

I'll update this in the doc, thanks for raising this.

BTW you can submit a PR for my doc here...

https://github.com/lupyuen/pinetime-rust-mynewt/blob/master/rust/app/src/simulator.md

lupyuen commented 4 years ago

@ZephyrLabs We spoke about marking the sections in Clock.cpp, now might be a good time to revisit this :-)

//  Define Your Widgets Here
...
//  Create Your Widgets Here
...
//  Refresh Your Widgets Here
...
ZephyrLabs commented 4 years ago

@lupyuen I shall submit a PR for marking those regions. but I believe the section could be more broadened with,

  1. normal refresh, using the Clock::refresh() function Independent of time change... (useful for things such as animations)

    2.Refresh based upon time change within the if(currentDateTime.IsUpdated()) clause (useful for things like the Analog clock(future), AM/PM time Indicators etc.)

lupyuen commented 4 years ago

@ZephyrLabs Thanks! :-) Have a chat with JF... See how best we can guide our newbies

ZephyrLabs commented 4 years ago

No problem, please tag me, I will respond as soon as possible :-)

lupyuen commented 4 years ago

FYI I'm working on a new Simulator for Rust on Mynewt... Hopefully the Rust Compiler will do better validation and prevent such Sandbox problems.

The watch face gets compiled as a separate Rust Library and linked into the PineTime Firmware, also into the Simulator, like this...

https://crates.io/crates/barebones-watchface

This also allows watch faces to be shared easily as Rust Libraries.

ZephyrLabs commented 4 years ago

@JF002 which branch can I leave a PR for the marked sections of Clock.cpp ? I have a opinion of rearranging the entire Clock.cpp so that its both functional and acts as a barebones design template for future designs...

hrmckay commented 4 years ago

Thanks for the information. I should have read the documentation more carefully. I was going to close this issue but you seem to be using it as a message board so I will leave it open.

Hugh

JF002 commented 4 years ago

@ZephyrLabs Please submit your PRs to the develop branch (https://github.com/JF002/Pinetime/blob/master/doc/contribute.md). Thanks!

ZephyrLabs commented 4 years ago

@JF002 I have added commented regions for addition of new widgets, Pull request to Develop Branch created, please continue the topic from there, Thank you! #90

lupyuen commented 4 years ago

Thanks that should be helpful :-)

ZephyrLabs commented 4 years ago

welcome :-)