search

lancaster-university / microbit-dal

http://lancaster-university.github.io/microbit-docs
Other
256 stars 130 forks source link

Update to allow mbed and the mbed online compiler to build. #449

Open GeorgeWort opened 5 years ago

GeorgeWort commented 5 years ago

Update to allow mbed and the mbed online compiler to build. This allows gcc_arm, armcc, and armclang to build the DAL.

Most changes are my personal changes in order to comply with the restrictions of the up to date compilers, and are meant to give equivalent behaviour to previously.

microbit-sam commented 5 years ago

Linked to: nrf51-sdk: https://github.com/lancaster-university/nrf51-sdk/pull/1 mbed-classic: https://github.com/lancaster-university/mbed-classic/pull/13 microbit-dal: https://github.com/lancaster-university/microbit-dal/pull/449

ghost commented 5 years ago

Not looked at the detail but I see quite a lot of files have changed and I'm wondering what regression testing has been performed / will be performed? I note the changes are "meant to give equivalent behaviour".... but would feel reassured if this had been proven. Thanks.

martinwork commented 5 years ago

In places, this appears to be a development from an earlier version of the DAL. It looks like that's the only reason for the MicroBitBLEManager changes, for example. Other files are purely white space changes. Should we be adding .lib files? If they are necessary, maybe this should be in a branch or fork repo. Will it still compile with yotta? Why replace temporary arrays on the stack with new/delete heap allocations? It's hard to make out what the essential changes are.

jamesadevine commented 5 years ago

I'm a bit nervous about most of these changes. I think these will require extensive regression tests. @jaustin

GeorgeWort commented 5 years ago

The scope of this PR will be reduced to only include the changes required for ARMC6 compatibility.

martinwork commented 5 years ago

Could the changes for each syntax compatibility issue be grouped, so we can record what syntax to avoid in the future? Maybe a separate PR for the changes needed for the build system?

GeorgeWort commented 5 years ago

Commits https://github.com/lancaster-university/microbit-dal/pull/449/commits/5b015a324cdb70e6a548110bbd704fcb4de95b71 and https://github.com/lancaster-university/microbit-dal/pull/449/commits/4f1a31fa2236f322e31bf910fa1066509b126e39 are enough to allow mbed-cli to work with gcc-arm.