Allow CONFIG_BT_NIMBLE_MAX_CONNECTIONS to be defined by project

[fvanroie]

CONFIG_BT_NIMBLE_MAX_CONNECTIONS is defined solely in the nimconfig.h header of the library. This requires a change to the library file to increase the number of connections. To make it more portable, I would like to define this macro in the bluild_flags of my PlatformIO project, like so:

build_flags =
    -D CONFIG_NIMBLE_CPP_ENABLE_RETURN_CODE_TEXT
    -D CONFIG_BT_NIMBLE_MAX_CONNECTIONS=9

This currently generates a compiler warning, because it's already defined:

.pio/libdeps/lolind32pro-lolintft24/NimBLE-Arduino/src/nimconfig.h:105:0: warning: "CONFIG_BT_NIMBLE_MAX_CONNECTIONS" 
redefined #define CONFIG_BT_NIMBLE_MAX_CONNECTIONS 3

It would be nice if the #define in the nimconfig.h header would be enclosed by #ifndef checks:

#ifndef CONFIG_BT_NIMBLE_MAX_CONNECTIONS
#define CONFIG_BT_NIMBLE_MAX_CONNECTIONS 3
#endif

This way, the macro can be set in the project without the need to touch any of the library files. This is especially handy when using the platformio library version of NimBLE-Arduino. The change will be transparent for users that don't use build flags.

[renatobo]

There might be some confusion around the usage of MYNEWT_VAL_BLE_MAX_CONNECTIONS which is overriding CONFIG_BT_NIMBLE_MAX_CONNECTIONS in a few locations

[h2zero]

@fvanroie thank you for the suggestion, this is a good idea.

@renatobo MYNEWT_VAL_BLE_MAX_CONNECTIONS is defined as the CONFIG_BT_NIMBLE_MAX_CONNECTIONS value. Unless you've seen an instance where it is not I don't see any issue. Could you elaborate?

[fvanroie]

I could not get more clients to connect using MYNEWT_VAL_BLE_MAX_CONNECTIONS.

The only workaround I have found is to specify a CONFIG_BT_NIMBLE_TASK_STACK_SIZE to circumvent the Arduino portion in the nimconfig.h file. This also requires all 30-something macros to be defined in my project, and not just CONFIG_BT_NIMBLE_MAX_CONNECTIONS. Using #ifndef it would be very easy to specify from outside the library.

For my project I only want to increase the number of clients, but maybe other settings could benefit from being configurable using preprocessor flags too...

[h2zero]

Yes, nimconfig.h was designed such that Arduino users could set the config settings and still have compatibility with the NimBLE stack in IDF which has the defines already in sdkconfig. It would make sense to support platformio in a similar fashion.

MYNEWT_VAL_BLE_MAX_CONNECTIONS doesn't work for the C++ wrapper because the code does not check that value, only CONFIG_BT_NIMBLE_MAX_CONNECTIONS.

Ideally all the settings could be configured in this way, I'm not sure how valuable that would be though. It would start making the nimconfig.h a bit messy.

[fvanroie]

Maybe it's best to start with the few ones that make sense, based on the likelihood they need to be changed from the defaults.
While max connections is an obvious one, I'm not even sure what most the other ones do... so that one would suffice for me.

Some macros are commented by default, so they can already be set using flags when needed without touching the file.

[renatobo]

@renatobo MYNEWT_VAL_BLE_MAX_CONNECTIONS is defined as the CONFIG_BT_NIMBLE_MAX_CONNECTIONS value. Unless you've seen an instance where it is not I don't see any issue. Could you elaborate?

Your comment 'MYNEWT_VAL_BLE_MAX_CONNECTIONS doesn't work for the C++ wrapper' clarified thanks!

I suggest allowing external defines for the following values as they can help reducing flash usage

• CONFIG_BT_NIMBLE_ROLE_CENTRAL (Client)
• CONFIG_BT_NIMBLE_ROLE_PERIPHERAL (Server)

while

• CONFIG_BT_NIMBLE_ROLE_BROADCASTER (Advertising)
• CONFIG_BT_NIMBLE_ROLE_OBSERVER (Scan)
  are not required as they are later defined if the previous two (Client, Server role) are defined.

[h2zero]

@renatobo Thank you for that suggestion as well. It makes sense to put conditionals around those as well.

[h2zero]

Just pushed the changes. Please let me know if I missed any.

[fvanroie]

Thanks, this handles my use case for setting CONFIG_BT_NIMBLE_MAX_CONNECTIONS .

For disabling a role that is set be default, I think the logic needs to be reversed or turned into an 0/1 value instead.
For example these roles are always defined, whether or not you define them in your project:

/** @brief Comment out if not using NimBLE Client functions \n
 *  Reduces flash size by approx. 7kB.
 */
#ifndef CONFIG_BT_NIMBLE_ROLE_CENTRAL
#define CONFIG_BT_NIMBLE_ROLE_CENTRAL
#endif

If the goal is to allow it to be disabled in the project, I would suggest something like:

#ifndef CONFIG_BT_NIMBLE_ROLE_CENTRAL
#define CONFIG_BT_NIMBLE_ROLE_CENTRAL 1
#endif

Then checking for CONFIG_BT_NIMBLE_ROLE_CENTRAL ==1 to actually enable it.

[h2zero]

Haha yes, you're right, wasn't really thinking when I made that change. Thanks!

[h2zero]

I ended up just renaming the macros for the command line by appending _DISABLED. If I had values set instead I would have to make many changes to the core files.

[fvanroie]

Indeed, that should work too and keeps the rest of the flow in line. Thanks!

[h2zero]

Closing this as resolved.