facchinm
commented
3 years ago @landret thanks for submitting this! @per1234 can we fix the CI before merging?
Closed landret closed 3 years ago
facchinm
commented
3 years ago @landret thanks for submitting this! @per1234 can we fix the CI before merging?
per1234
commented
3 years ago can we fix the CI before merging?
@facchinm the fix is all ready only waiting to be merged: https://github.com/arduino/ArduinoCore-megaavr/pull/98
landret
commented
3 years ago can we fix the CI before merging?
@facchinm the fix is all ready only waiting to be merged:
98
All the check failed on 27th April with this error (don't know if it was after the CI fix merging):
Warning: This version of the action is deprecated. Use arduino/compile-sketches. See https://github.com/arduino/compile-sketches
per1234
commented
3 years ago @landret the CI has been fixed but the pull request needs to be rebased on top of it.
landret
commented
3 years ago @landret the CI has been fixed but the pull request needs to be rebased on top of it.
Thanks for the reply. How do I rebase it? Do I have to do a new pull request?
per1234
commented
3 years ago @landret no, it's not necessary to do a new pull request. You can see a tutorial about it here:
https://akrabat.com/the-beginners-guide-to-rebasing-your-pr/
Note that in the tutorial the pull request base branch is named develop, but the base branch of your pull request is named master.
It's not possible to do this process via the GitHub web interface. The tutorial is for using Git from the command line. Some Git clients also provide the ability to do it via a GUI, but that process will depend on which Git client you happen to be using.
Please let me know if you have any problems.
landret
commented
3 years ago @landret no, it's not necessary to do a new pull request. You can see a tutorial about it here:
https://akrabat.com/the-beginners-guide-to-rebasing-your-pr/
Note that in the tutorial the pull request base branch is named
develop, but the base branch of your pull request is namedmaster.It's not possible to do this process via the GitHub web interface. The tutorial is for using Git from the command line. Some Git clients also provide the ability to do it via a GUI, but that process will depend on which Git client you happen to be using.
Please let me know if you have any problems.
Done, thanks. Now it looks like the workflow needs approval for running here.
per1234
commented
3 years ago Great work @landret!
The workflow needing approval is the result of a recent policy change from GitHub to prevent malicious abuse of GitHub Actions, and not specific to this PR or the repository. I don't have the necessary permissions here to approve the workflow runs. Maybe @facchinm can help us out with that.
github-actions[bot]
commented
3 years ago Memory usage change @ 991866d0be7f6c30ced5c4545f4b1e302323a15c
| Board | flash | % | RAM for global variables | % |
|---|---|---|---|---|
| arduino:megaavr:nona4809 | 0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
facchinm
commented
3 years ago CI output is a bit difficult to interpret but it's failing on Tweeter example from https://github.com/arduino-libraries/Arduino_OAuth/tree/master/examples/Tweeter lib due to low resources. @per1234 should we exclude that example (or library) from the CI?
per1234
commented
3 years ago CI output is a bit difficult to interpret
For me, there is an excessive amount of output. I would propose removing the "verbose output during compilation" setting from the CI workflow: https://github.com/arduino/ArduinoCore-megaavr/blob/1c265e4d7095ee725d7d9d274f7060b98157e3d0/.github/workflows/compile-examples.yml#L148 That option is primarily intended for use when troubleshooting the action or build system.
I did that in the SAMD boards platform repo a couple months ago (https://github.com/arduino/ArduinoCore-samd/pull/617) and haven't heard any complaints from people missing the verbose output.
should we exclude that example (or library) from the CI?
If the sketch is intended to be usable with these boards, and the work needed to restore usability is related to this repository's code, then I would say it should stay in order to track the problem. If the sketch is not intended to be usable with these boards, then it should be removed.
I'm happy to adjust the CI configuration according to your preferences. Just let me know.
facchinm
commented
3 years ago I'm super ok with removing the verbosity; for the example, I think it started failing for external reasons due to some dependency becoming too resource hungry. The possible culprits are
2021-04-20T08:34:26.8082157Z Using library ArduinoECCX08 at version 1.3.5 in folder: /home/runner/Arduino/libraries/ArduinoECCX08
2021-04-20T08:34:26.8082995Z Using library Wire at version 1.0 in folder: /home/runner/.arduino15/packages/arduino/hardware/megaavr/1.8.7/libraries/Wire
2021-04-20T08:34:26.8083872Z Using library ArduinoBearSSL at version 1.7.0 in folder: /home/runner/Arduino/libraries/ArduinoBearSSL
2021-04-20T08:34:26.8084765Z Using library ArduinoHttpClient at version 0.4.0 in folder: /home/runner/Arduino/libraries/ArduinoHttpClient
2021-04-20T08:34:26.8085671Z Using library Arduino_OAuth at version 0.1.0 in folder: /home/runner/Arduino/libraries/Arduino_OAuth
2021-04-20T08:34:26.8086400Z Using library WiFiNINA at version 1.8.7 in folder: /home/runner/Arduino/libraries/WiFiNINA WiFiNINA is the only one that was heavily updated lately, and maybe the ArduinoBearSSL dependency is a side effect of a commit there. Investigating :detective:
facchinm
commented
3 years ago So, after bisecting, it looks like the culprit is this PR https://github.com/arduino-libraries/ArduinoBearSSL/pull/34 ; probably the avr compile is not smart enough to strip the singletons (MD5, AES128 and so on) so they become part of the binary even if unused.
@aentinger any hint on how we could solve both the problems at once?
aentinger
commented
3 years ago Not sure. We've got -flto, -ffunction-sections -fdata-sections, -Wl,--gc-sections. That should do the trick in theory. Can confirm overflow with ArduinoBearSSL, here's the map file: Tweeter.ino.map.txt
landret
commented
3 years ago Hi guys. Have the CI problems been solved ?
per1234
commented
3 years ago Regarding this proposal I mentioned above:
I would propose removing the "verbose output during compilation" setting from the CI workflow:
The PR is here: https://github.com/arduino/ArduinoCore-megaavr/pull/104
But that is only intended to make the logs easier to read to understand the cause of a failed compilation.
facchinm
commented
3 years ago The CI error should be fixed by https://github.com/arduino-libraries/ArduinoBearSSL/releases/tag/1.7.1 , so as soon as the library is picked up by the library manager we can restart the job and then merge the PR.
github-actions[bot]
commented
3 years ago Memory usage change @ 71437e7659812d763c88094f69a94ef2b0468ea3
| Board | flash | % | RAM for global variables | % |
|---|---|---|---|---|
| arduino:megaavr:nona4809 | 0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
| arduino:megaavr:uno2018:mode=off | 0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
| arduino:megaavr:uno2018:mode=on | 0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
landret
commented
3 years ago Hi @facchinm I rebased the PR on top and all checks have passed now.
Hi, based on issues #100 and to allow use of precompiled libraries, I added the same patch as https://github.com/arduino/ArduinoCore-avr/commit/bca2493a5c68ba5c0a2c6963b462d917794bfb2d to the platform.txt.
This patch was successfully tested with Nano Every and Uno Wifi Rev2.