Cleanup TARGET_NRF5 and TARGET_NRF5x #6711

marcuschangarm · 2018-04-23T19:33:22Z

Description

The unified NRF51 target and feature BLE directories have been
reorganized to follow the naming and directory structure of the
NRF52 implementation.

This reorganization does not include TARGET_MCU_NRF51822 and
derived targets.

Unused NRF52 files have been removed.

Pull request type

[ ] Fix
[x] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

marcuschangarm · 2018-04-23T19:43:48Z

@0xc0170
I didn't touch targets/TARGET_NORDIC/TARGET_MCU_NRF51822/ since the structure is incompatible with TARGET_MCU_NRF51822_UNIFIED.

@donatieng
I had to split out the NRF51 and NRF52 BLE implementation into separate directories since they use different storage implementations.

@bulislaw @mprse
I had to split out the clocks into separate directories in targets/TARGET_NORDIC/TARGET_NRF5x/ in order to make the NRF52 work. However, I expect we can merge them again when #5629 has been rebased?

0xc0170 · 2018-04-26T12:06:51Z

I had to split out the clocks into separate directories in targets/TARGET_NORDIC/TARGET_NRF5x/ in order to make the NRF52 work. However, I expect we can merge them again when #5629 has been rebased?

Which commit is this one, or reference on your branch to review?
I reviewed changes, looks fine to me.

Please rebase to resolve conflicts.

marcuschangarm · 2018-04-26T15:33:15Z

Which commit is this one, or reference on your branch to review?

bd8dd20

Both TARGET_NRF51 and TARGET_NRF52 contain ticker implementations. With #5629 we can combine the two into the same implementation.

adbridge · 2018-04-30T11:09:36Z

@bulislaw , @donatieng We are still awaiting your reviews on this PR, could you please do so asap.

pan- · 2018-04-30T11:53:50Z

I had to split out the NRF51 and NRF52 BLE implementation into separate directories since they use different storage implementations.

Could you detail this point ? Is it related to the peer manager used ?

marcuschangarm · 2018-04-30T15:20:34Z

@pan-

There is a big mismatch between whether the header files should have nrf_ prefixed or not, but the main issue is the new softdevice header:

https://github.com/marcuschangarm/mbed-os/blob/cleanup-nrf5x/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF51/source/btle/btle.cpp#L49-L50

https://github.com/marcuschangarm/mbed-os/blob/cleanup-nrf5x/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF52/source/btle/btle.cpp#L59-L60

I'm also not familiar enough with the NRF51 to make too many changes in one go.

marcuschangarm · 2018-05-02T15:02:41Z

@donatieng @bulislaw

Can you have a look please?

0xc0170 · 2018-05-03T08:42:50Z

It looks fine to me (SDK11 to SDK_11 change follows the rest of the naming ?)

There are conflicts to be resolved

marcuschangarm · 2018-05-03T18:35:42Z

/morph build

mbed-ci · 2018-05-03T19:12:54Z

Build : SUCCESS

Build number : 1917
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6711/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

marcuschangarm · 2018-05-03T19:50:27Z

It looks fine to me (SDK11 to SDK_11 change follows the rest of the naming ?)

Yes, that was from external input.

There are conflicts to be resolved

Done.

mbed-ci · 2018-05-04T03:53:53Z

Exporter Build : SUCCESS

Build number : 1562
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6711/

mbed-ci · 2018-05-04T16:28:29Z

Test : FAILURE

Build number : 1733
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6711/1733

0xc0170 · 2018-05-07T09:47:01Z

One test fails for all toolchains, related to the changes?

We have a conflict

cmonr · 2018-05-07T16:05:39Z

This also needs a rebase.

Superseded by new SDK 14.2 in #6547

marcuschangarm · 2018-05-07T19:23:04Z

/morph build

marcuschangarm · 2018-05-07T19:23:42Z

One test fails for all toolchains, related to the changes?

I only moved the file to a different folder, I didn't change it.

mbed-ci · 2018-05-07T20:16:42Z

Build : SUCCESS

Build number : 1938
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6711/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-05-08T05:55:19Z

Exporter Build : SUCCESS

Build number : 1584
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6711/

mbed-ci · 2018-05-08T06:29:41Z

Test : FAILURE

Build number : 1760
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6711/1760

mprse · 2018-05-08T08:22:01Z

I checked tests-mbed_hal-critical_section test and it uses special logic for NRF boards to check if interrupts are locked. For NRF51_DK the logic is based on TARGET_NRF5 symbol.
The problem is that on marcuschangarm:cleanup-nrf5x branch TARGET_NRF5 symbol does not exist.
I compared targets.json file on master and marcuschangarm:cleanup-nrf5x branch and it looks like TARGET_NRF5 has been replaced by TARGET_NRF5x and TARGET_NRF51 symbols.

So, it looks like the test needs to be updated. In the code below TARGET_NRF5 should be replaced probably with TARGET_NRF5x (checked that this change solves the problem on NRF51_DK board):

mbed-os/TESTS/mbed_hal/critical_section/main.cpp

Lines 36 to 39 in 11901f1

    
           #elif TARGET_NRF5 
        
           	// check if APP interrupts are masked for other NRF5 boards 
        
           	return ((NVIC->ISER[0] & __NRF_NVIC_APP_IRQS_0) != 0); 
        
           #else

marcuschangarm · 2018-05-08T16:59:14Z

Good catch! Thank you!

The unified NRF51 target and feature BLE directories have been reorganized to follow the naming and directory structure of the NRF52 implementation. This reorganization does not include TARGET_MCU_NRF51822 and derived targets.

marcuschangarm · 2018-05-08T17:12:00Z

/morph build

mbed-ci · 2018-05-08T17:21:51Z

Build : FAILURE

Build number : 1948
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6711/

mbed-ci · 2018-05-08T17:47:49Z

Build : SUCCESS

Build number : 1949
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6711/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-05-08T20:03:43Z

Test : SUCCESS

Build number : 1769
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6711/1769

mbed-ci · 2018-05-09T03:38:33Z

Exporter Build : FAILURE

Build number : 1601
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6711/

marcuschangarm · 2018-05-09T05:01:27Z

/morph export-build

mbed-ci · 2018-05-09T11:25:29Z

Exporter Build : FAILURE

Build number : 1607
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6711/

marcuschangarm · 2018-05-09T14:54:24Z

/morph export-build

mbed-ci · 2018-05-09T19:10:04Z

Exporter Build : FAILURE

Build number : 1614
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6711/

marcuschangarm · 2018-05-10T20:14:19Z

/morph export-build

mbed-ci · 2018-05-10T22:51:20Z

Exporter Build : SUCCESS

Build number : 1623
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6711/

marcuschangarm · 2018-05-10T23:29:22Z

@theotherjimmy Fourth time is indeed the charm! 😄

cmonr

That was much less painful than I was expecting.

👍 on using git mv commands.

Reintroduce PR #6021 #6021 which was accidentally removed by PR #6711 #6711

0xc0170 added the needs: review label Apr 24, 2018

0xc0170 requested review from bulislaw and donatieng April 24, 2018 08:33

0xc0170 added needs: work and removed needs: review labels Apr 26, 2018

adbridge added the release-version: 5.9.0-rc1 label Apr 30, 2018

Remove unused NRF52 SDK and MCU files

1360920

Superseded by new SDK 14.2 in #6547

Reorganize TARGET_MCU_NRF51822_UNIFIED directories

1aebdcb

The unified NRF51 target and feature BLE directories have been reorganized to follow the naming and directory structure of the NRF52 implementation. This reorganization does not include TARGET_MCU_NRF51822 and derived targets.

cmonr added needs: review and removed needs: work labels May 11, 2018

cmonr approved these changes May 11, 2018

View reviewed changes

cmonr added ready for merge and removed needs: review labels May 11, 2018

cmonr merged commit 2104d8a into ARMmbed:master May 12, 2018

tkstreet mentioned this pull request May 15, 2018

Cannot compile with O0 optimization [nRF52-DK] [GCC-ARM] #6920

Closed

cmonr removed the ready for merge label May 21, 2018

marcuschangarm mentioned this pull request May 25, 2018

Fix array overflow in gpio configuration code for NRF5x #7022

Merged

marcuschangarm deleted the cleanup-nrf5x branch May 25, 2018 18:00

adbridge pushed a commit that referenced this pull request Jun 15, 2018

Fix array overflow in gpio configuration code for NRF5x

28060c1

Reintroduce PR #6021 #6021 which was accidentally removed by PR #6711 #6711

Cleanup TARGET_NRF5 and TARGET_NRF5x #6711

Cleanup TARGET_NRF5 and TARGET_NRF5x #6711

Uh oh!

Conversation

marcuschangarm commented Apr 23, 2018 • edited by adbridge Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Uh oh!

marcuschangarm commented Apr 23, 2018

Uh oh!

0xc0170 commented Apr 26, 2018

Uh oh!

marcuschangarm commented Apr 26, 2018

Uh oh!

adbridge commented Apr 30, 2018

Uh oh!

pan- commented Apr 30, 2018

Uh oh!

marcuschangarm commented Apr 30, 2018

Uh oh!

marcuschangarm commented May 2, 2018

Uh oh!

0xc0170 commented May 3, 2018

Uh oh!

marcuschangarm commented May 3, 2018

Uh oh!

mbed-ci commented May 3, 2018

Triggering tests

Uh oh!

marcuschangarm commented May 3, 2018

Uh oh!

mbed-ci commented May 4, 2018

Uh oh!

mbed-ci commented May 4, 2018

Uh oh!

0xc0170 commented May 7, 2018

Uh oh!

cmonr commented May 7, 2018

Uh oh!

marcuschangarm commented May 7, 2018

Uh oh!

marcuschangarm commented May 7, 2018

Uh oh!

mbed-ci commented May 7, 2018

Triggering tests

Uh oh!

mbed-ci commented May 8, 2018

Uh oh!

mbed-ci commented May 8, 2018

Uh oh!

mprse commented May 8, 2018

Uh oh!

marcuschangarm commented May 8, 2018

Uh oh!

marcuschangarm commented May 8, 2018

Uh oh!

mbed-ci commented May 8, 2018

Uh oh!

mbed-ci commented May 8, 2018

Triggering tests

Uh oh!

mbed-ci commented May 8, 2018

Uh oh!

mbed-ci commented May 9, 2018

Uh oh!

marcuschangarm commented May 9, 2018

Uh oh!

mbed-ci commented May 9, 2018

Uh oh!

marcuschangarm commented May 9, 2018

Uh oh!

mbed-ci commented May 9, 2018

Uh oh!

marcuschangarm commented May 10, 2018

Uh oh!

mbed-ci commented May 10, 2018

Uh oh!

marcuschangarm commented May 10, 2018

Uh oh!

marcuschangarm commented Apr 23, 2018 •

edited by adbridge

Loading