52840/sdk15 boards - no output over BLE without having -DCENTRAL_LINK_COUNT=1 in board file

[fanoush]

This does not work for some reason
https://github.com/espruino/Espruino/blob/62acf92dd9cd718bfb4c1c013f67b0c80fc1a489/targets/nrf5x/app_config.h#L181C3-L181C3

with this patch

diff --git a/boards/BANGLEJS2.py b/boards/BANGLEJS2.py
index 91641c0cf..1494e0797 100644
--- a/boards/BANGLEJS2.py
+++ b/boards/BANGLEJS2.py
@@ -47,7 +47,7 @@ info = {
      #'DEFINES += -DESPR_REGOUT0_1_8V=1', # this increases power draw, so probably not correct!
      'DEFINES += -DESPR_LSE_ENABLE', # Ensure low speed external osc enabled
      'DEFINES += -DNRF_SDH_BLE_GATT_MAX_MTU_SIZE=131', # 23+x*27 rule as per https://devzone.nordicsemi.com/f/nordic-q-a/44825/ios-mtu-size-why-only-185-bytes
-     'DEFINES += -DCENTRAL_LINK_COUNT=2 -DNRF_SDH_BLE_CENTRAL_LINK_COUNT=2', # allow two outgoing connections at once
+#     'DEFINES += -DCENTRAL_LINK_COUNT=2 -DNRF_SDH_BLE_CENTRAL_LINK_COUNT=2', # allow two outgoing connections at once
      'LDFLAGS += -Xlinker --defsym=LD_APP_RAM_BASE=0x3660', # set RAM base to match MTU=131 + CENTRAL_LINK_COUNT=2
      'DEFINES += -DESPR_DCDC_ENABLE=1', # Use DC/DC converter
      'ESPR_BLUETOOTH_ANCS=1', # Enable ANCS (Apple notifications) support
diff --git a/targets/nrf5x/app_config.h b/targets/nrf5x/app_config.h
index 40163138a..74ab1e04f 100644
--- a/targets/nrf5x/app_config.h
+++ b/targets/nrf5x/app_config.h
@@ -182,6 +182,7 @@
 #define CENTRAL_LINK_COUNT 1
 #else
 #define CENTRAL_LINK_COUNT 0
+#error Central is 0
 #endif
 #endif
 // SDK15+ (fixes BLE UART send when CENTRAL_LINK_COUNT>1)

I get Espruino/targets/nrf5x/app_config.h:185:2: error: #error Central is 0 when building for BANGLEJS2

Not sure why PEER_MANAGER_ENABLED is undefined there, it is also tested in NRF5X_SDK_12 section above and that maybe works?

Noticed because I have older board files without the -DCENTRAL_LINK_COUNT lines and with fresh build I see no console output when connecting and also process.env.APP_RAM_BASE.toString(16) was lower than expected. Adding explicit
'DEFINES += -DCENTRAL_LINK_COUNT=1 -DNRF_SDH_BLE_CENTRAL_LINK_COUNT=1', helped.

[fanoush]

Oh, it works for SDK12 because PEER_MANAGER_ENABLED is actually defined few lines above the test just for SDK12!
Not sure if PEER_MANAGER_ENABLED test should be removed for SDK15 as there is no nrf51 with SDK15 anyway or the PEER_MANAGER_ENABLED=1 should be defined there for everything nrf52 and SDK >=12 (as SDK11 and lower does not have it)

[fanoush]

Oh, I see you moved it from SDK12 part in e51db23

I guess the test was there because of microbit1/nrf51 and not peer manager so it can be removed for SDK15? Created PR with this change.

[gfwilliams]

Thanks for tracking this down! I guess my only concern is does this leave PEER_MANAGER_ENABLED as undefined?

I assume it's probably set somewhere or it likely wouldn't build (maybe it's a default in sdk_config.h), but...

or the PEER_MANAGER_ENABLED=1 should be defined there for everything nrf52 and SDK >=12

I think it probably makes sense doing that to be more explicit, as I think the peer manager is needed for central mode...

[fanoush]

Yes it looks undefined, this

diff --git a/targets/nrf5x/bluetooth.c b/targets/nrf5x/bluetooth.c
index 288c799ad..21ab91143 100644
--- a/targets/nrf5x/bluetooth.c
+++ b/targets/nrf5x/bluetooth.c
@@ -69,6 +69,8 @@
 #if NRF_SD_BLE_API_VERSION<5
 #include "fstorage.h"
 #include "fstorage_internal_defs.h"
+#else
+#error PEERMANAGER UNDEFINED
 #endif
 #include "ble_conn_state.h"
 static pm_peer_id_t   m_peer_id;                              /**< Device reference handle to the current bonded central. */

added here
https://github.com/espruino/Espruino/blob/master/targets/nrf5x/bluetooth.c#L72
breaks banglejs2 build.

I think the peer manager is needed for central mode.

I think it is needed for bonding and secure conenctions so if this is not used, Peer Manager it is not needed, however it looks like currently it works for BANGLEJS2 insludin bonding without this PEER_MANAGER_ENABLED define?

Anyway I'd say CENTRAL_COUNT value is unrelated to peer manager being on/off even if currently missing PEER_MANAGER_ENABLED for SDK!=12 may be a bug.

[fanoush]

Oh, no, sorry, forget it that is wrong #endif, so looks like PEER_MANAGER_ENABLED is defined

[gfwilliams]

Ok, thanks for checking that out! Is PEER_MANAGER_ENABLED still defined in your SDK15 build without CENTRAL_LINK_COUNT=1 defined?

As you say peer manager may not be needed - however it's possible things may behave a bit strangely without it. I've never tested central with it not enabled

[fanoush]

Is PEER_MANAGER_ENABLED still defined in your SDK15 build without CENTRAL_LINK_COUNT=1 defined?

Yes. When just commenting out 'DEFINES += -DCENTRAL_LINK_COUNT=2 -DNRF_SDH_BLE_CENTRAL_LINK_COUNT=2' in BANGLEJS2 board and adding the #else and #error to line https://github.com/espruino/Espruino/blob/master/targets/nrf5x/bluetooth.c#L72 as mentioned in previous post it prints the error.
And at that place it is inside PEER_MANAGER_ENABLED and NRF_SD_BLE_API_VERSION>=5. So by mistake I put it to right place to test exactly this.

It probably gets defined later from this place https://github.com/espruino/Espruino/blob/master/targetlibs/nrf5x_15/nrf52_config/sdk_config.h#L120

[gfwilliams]

Thanks! Ok, that's good then. I just added a line in to set PEER_MANAGER in app_config - I know it's not needed but I think it might be helpful to have it all in one place