search

ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.66k stars 2.97k forks source link

Multiple definitions of hal_sleep and hal_deepsleep for target DELTA_DFCM_NNN40 #3672

Closed sarahmarshy closed 7 years ago

sarahmarshy commented 7 years ago

Description

Bug

Target DELTA_DFCM_NNN40

Toolchain: GCC_ARM|ARM|IAR

mbed-cli version: 1.0.0

meed-os sha: 450701f211df4de358507f8d490f0c35504d6697

Steps to reproduce I was using the ci-test shield, but may be reproducible with anything using the ble feature.

The conflict arises between sleep definitions in FEATURE_BLE and the target hal.

Sleep defintion in feature ble: https://github.com/ARMmbed/mbed-os/blob/master/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/hal_patch/sleep.c#L26

Sleep definition in the target hal: https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/sleep.c#L21

I can fix the compilation error by reverting this change in the ble feature. However, what is intended here? Should the sleep function be different when BLE is in use?

@bulislaw & @0xc0170 I see that you recently made changes to these sleep api @pan- for ble

bulislaw commented 7 years ago

Ah that's the weak definition we removed, now it all make sense. I'll push a fix in next 5 minutes.

pan- commented 7 years ago

I can fix the compilation error by reverting this change in the ble feature. However, what is intended here? Should the sleep function be different when BLE is in use?

@sarahmarshy Yes, the sleep function is different when ble is in use but the definition of the sleep function in BLE is here for legacy reason and compatibility reasons with mbed OS 2. The old implementation of the NORDIC HAL doesn't include the Nordic SDK which is necessary to declare a single and clean implementation of the sleep function on NRF targets, instead the SDK was distributed as part of the BLE implementation therefore it was the only place a sleep implementation could be declared. Once all targets will have migrated to the new HAL implementation then it will become possible to remove this hideous exhibit of monkey patching. I don't expect it to happen any time soon.

@bulislaw In case of question about Nordic targets you can invoke @nvlsianpu @anangl or myself.