search

AdaCore / bb-runtimes

Source repository for the GNAT Bare Metal BSPs
Other
65 stars 51 forks source link

Added NRF52833 (MicrobitV2) Ravenscar and Jorvik support #63

Closed aiunderstand closed 1 year ago

aiunderstand commented 1 year ago

To build run: python .\build_rts.py --output=C:\GNAT\2021-arm-elf\arm-eabi\lib\gnat --build nrf52833 --force

Requirements:

Notes: Tested runtimes:

NOT working (see some relevant discussion in ADL PR for MicrobitV2)

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

aiunderstand commented 1 year ago

All in all it looks good. I commented on a few small style issues.

Thanks for the detailed feedback @jklmnn . All suggested changes are committed.

aiunderstand commented 1 year ago

Sorry, I deleted the files with unrelated changes instead of removing my style changes. I will add them back and make sure to test correct building of the runtimes using the command listed at the top of this PR

aiunderstand commented 1 year ago

This PR, which enables ravenscar and jorvik support for the NRF52833 (eg. Micro:Bit v2) has now integrated the style suggestions, re-enabled semi-hosting and removed the s-textio file.

It builds the nrf52833 runtimes using the command stated above resulting in:

image

Note that the runtimes are created using the 2021 toolchain

jklmnn commented 1 year ago

Note that the runtimes are created using the 2021 toolchain

Do you specifically need these changes for the 2021 toolchain? Otherwise I would merge these changes into master so they will be available in upcoming releases.

aiunderstand commented 1 year ago

Sorry, I meant to say I have only tested the proposed nrf52833 runtimes for the 2021 toolchain. I would very much like to see the profiles available into the master repo but don't know if there are any breaking changes.

aiunderstand commented 1 year ago

@jklmnn @Fabien-Chouteau I just tried to build the runtime with a merged master in my local repo but the command to build does not work anymore. There has been a change in installer.py now requiring a version (failing in line 51). What syntax should I use?

jklmnn commented 1 year ago

This repository is mirrored from our internal one. Unfortunately the master branch in this repository is not up to date at the moment but I'm already working on that. I will put your changes through our internal review (I don't expect any major changes there) and if you're okay with it I would take care of rebasing your changes to the latest master and merge them. Your commits should then pop up here.

aiunderstand commented 1 year ago

Thanks, I look forward to your commit.

jklmnn commented 1 year ago

I just noticed you were using merge commits within the git history of this PR. ~We can only merge changes that apply cleanly to our master branch. Are you okay with me cleaning up the git history of your changes (remove the merge commits, squash style fixes)? Your name and email address will of course stay in these commits.~ Can you please squash your changes into a single commit?

aiunderstand commented 1 year ago

Ofcourse! Apologies for the extra work.

On Tue, Sep 26, 2023, 15:35 JK @.***> wrote:

I just noticed you were using merge commits within the git history of this PR. We can only merge changes that apply cleanly to our master branch. Are you okay with me cleaning up the git history of your changes (remove the merge commits, squash style fixes)? Your name and email address will of course stay in these commits.

— Reply to this email directly, view it on GitHub https://github.com/AdaCore/bb-runtimes/pull/63#issuecomment-1735556690, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQUVP2QU6RM75IPRW2BQ3TX4LKYJANCNFSM6AAAAAAS3BR7WI . You are receiving this because you authored the thread.Message ID: @.***>

aiunderstand commented 1 year ago

I created a new branch nrf52833 on my repo forked from the latest upstream/community-2021 branch. I then added all files in a single commit, checked that it worked and submitted a new PR #67 .

Please don't add this PR (#63) as the commit history is made worse while trying to do the interactive rebase to squash the commits.. I did a git reset --hard to undo the interactive rebase. So the status is back to the squash request. What do I need to do squash them in this repo? Isnt it easier to integrate PR #67 where everything is squashed to one commit? Thanks!

aiunderstand commented 1 year ago

Hi @jklmnn is there something I need to do for this PR to be integrated?

jklmnn commented 1 year ago

Hi @jklmnn is there something I need to do for this PR to be integrated?

Unfortunately I'm quite busy these days, I'll try to look as soon as possible at https://github.com/AdaCore/bb-runtimes/pull/67. Are you fine with closing this one so we don't have duplicates?

aiunderstand commented 1 year ago

Hi @jklmnn is there something I need to do for this PR to be integrated?

Unfortunately I'm quite busy these days, I'll try to look as soon as possible at #67. Are you fine with closing this one so we don't have duplicates?

Of course!

jklmnn commented 1 year ago

Closed in favor of #67.