Apollo3: Fix heap size formula and stack start address in scatter file #14132
Conversation
ciarmcom
added
the
release-type: patch
|
@hugueskamba, thank you for your changes. |
Jenkins CI Test : ❌ FAILEDBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
Jenkins error, CI restarted |
Jenkins CI Test : ❌ FAILEDBuild Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
CI restarted |
Jenkins CI Test : ❌ FAILEDBuild Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
I've asked test team to help reviewing logs to find what failed (the targets are related so it might be real failures but I could not locate them in the console/logs). |
|
|
Error is due to the random |
The heapsize was incorrectly calculated. This fixes it by subtracting the Stack size, any memory chunks allocated before the start of the application (for vectors and/or crash report), and finally the size of the application from the total RAM size. The stack start address should be the top of the RAM which is also fixed.
hugueskamba
force-pushed
the
hk_heap_size_fix_apollo3
branch
from
7f0ef5e to
f00aeea
Compare
hugueskamba
changed the title
Thanks. This has now been fixed. |
|
CI started |
|
CI restarted |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 7 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
mbedmain
added
release-version: 6.7.0
Release-pending
and removed
release-type: patch
|
This merge should be rolled back ASAP! Since the release of V6.7 i was updating my code to work again with MAX32630FTHR... Then today turning back to the Apollo3 Thing Plus board i face some of my code not working with the Artemis board and Mbed V6.7... OK, it might be my code, so i try the official blinky which is also very basic...even that fails with V6.7. So i go and look for what has been changed for the Apollo3 platform in V6.7. Luckily i found only this PR. And everything works again. May i just question how all this PR has been tested? Has it been tested at all on the target itself? |
|
Did this break stack alignment ? We shall fix this in the upcoming release.
Not every board is plugged in CI, partners test the changes themselves or users should. In this case, the change was not tested locally. @ARMmbed/team-sparkfun ^^ |
Ok, i understand. I know you are making up/introduce working groups for the public. Why don't you try to organize such groups of users of different boards who could then test those boards physically? I only have 2 boards supported by Mbed, but as i perceive there is a general tendency that vendors just put their products to the market and they make an initial effort to get it Mbed enabled, but after then they almost abandon their products. Combined with the fast pace how Mbed itself evolves it leads to no good. I do not know how you involve new vendors, but maybe i would require them to have ongoing commitment to support their boards. Here i do not mean they shall find and fix all the bugs on their own, but i find it eye-opening how many times they just do not react on approval requests. Mbed already supports tons of different platforms, you do not need to support all the boards. For QA reasons you could simply drop those boards where the vendor does not review PRs before merging and it must be clear to them from the beginning of the partnership. However you should take care and think of the reasons why vendors tend to act like that. You really should not stay on the current path in this regard. |
|
thanks for the feedback, we will improve the process to set proper expectations and improve how targets are tested. |
Summary of changes
The heapsize was incorrectly calculated.
This fixes it by subtracting the Stack size, any memory chunks allocated
before the start of the application (for vectors and/or crash report), and
finally the size of the application from the total RAM size.
The stack start address should be the top of the RAM which is also fixed.
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers