search

ARMmbed / ble-nrf51822

Nordic stack and drivers for the mbed BLE_API
Other
46 stars 51 forks source link

Split nordic sdk into its own module #75

Closed LiyouZhou closed 8 years ago

LiyouZhou commented 8 years ago

Remove nordic files from this module and make ble-nrf51822 depend on nrf51-sdk. @rgrover

andresag01 commented 8 years ago

I wonder whether this is a good opportunity to remove the bootloder/ directory since it should actually be in the nrf51 "family" targets (see this pull request). However, if we are going to leave it here, I am unsure whether we need some sort of license file for it. @rgrover

rgrover commented 8 years ago

ble-nrf51822 is also shared with mbed-classic. The mbed-classic toolchain expects to find the bootloader here.

andresag01 commented 8 years ago

Ah ok, forgot about that

rgrover commented 8 years ago

happy with it in general.

LiyouZhou commented 8 years ago

@rgrover please review.

rgrover commented 8 years ago

ready for merge. I'll now check if the sources in nrf51-sdk match the files we're losing on ble-nrf51dk. What testing have you performed?

LiyouZhou commented 8 years ago

@rgrover broke ble-examples. hold off merging pending further investigation.

metc commented 8 years ago

@LiyouZhou IMO, splitting the Nordik SDK to its own module is great !

Should the nRF51 SDK be modified/patched from the original sources available here ? http://developer.nordicsemi.com/nRF51_SDK/nRF51_SDK_v8.x.x/

metc commented 8 years ago

Answer here, sorry... https://github.com/ARMmbed/nrf51-sdk/blob/master/README.md#changes-made-to-nordic-files

LiyouZhou commented 8 years ago

@rgrover I have made the fixes required. Please continue reviewing this merge.

rgrover commented 8 years ago

this a large collection of deletions, which is what I had expected it to be. is the nrf51-sdk module populated with exactly the same files which have been deleted from here?

LiyouZhou commented 8 years ago

@rgrover yes and more files used in other modules.

rgrover commented 8 years ago

published ble-nrf51822-2.2.0