BleClient Custom settings

[RobertoHE]

Class changed to new setting struct.
Some warnings appear when compile, but it works fine.
Please, @lathoub , check it before merge.

[lathoub]

Thanks @RobertoHE what sketch did you use to compile the code? (I get plenty of errors with your ble_client)
The BLE_Client points to BLEMIDI_NAMESPACE::DefaultSettings, whilst I believe that should be BLEMIDI_NAMESPACE:: DefaultSettingsClient (even then, I get compile errors)

https://github.com/RobertoHE/Arduino-BLE-MIDI/blob/fe3412b289ac60e09593304a9b73a62e5a03be81/examples/MidiBle_Client/MidiBle_Client.ino#L31

[RobertoHE]

That's true. Example code must use BLEMIDI_NAMESPACE:: DefaultSettingsClient as default struct.

If you change this, the example compiles with errors?

[lathoub]

If you change this, the example compiles with errors?

Yes, but after some re-arranging of the code I get it to compile:

#include <BLEMIDI_Transport.h>

#include <hardware/BLEMIDI_Client_ESP32.h>
//#include <hardware/BLEMIDI_ESP32_NimBLE.h>
//#include <hardware/BLEMIDI_ESP32.h>
//#include <hardware/BLEMIDI_ArduinoBLE.h>

struct CustomBufferSizeSettings : public BLEMIDI_NAMESPACE::DefaultSettingsClient {
   static const size_t MaxBufferSize = 16;
};

BLEMIDI_CREATE_CUSTOM_INSTANCE("Esp32-BLE-MIDI", MIDI, CustomBufferSizeSettings); // Connect to first server found

Can you also include the modified client in this PR please?

[RobertoHE]

Client example code modified.

[RobertoHE]

Is It possible to merge the pull request? If not, say me what changes it needs.

I think that struct settings are better than modifying defines directly in lib.
I think too that this configuration procedure may be the primary way to do that and maybe merge in the main branch in the next major release and deprecate the define way to the historical version in the previous release.

[lathoub]

Correct, it might be better the use the settings!
I made changes to the BLEMIDI_ArduinoBLE.h that I have just uploaded - can you check if this works on your end?

[RobertoHE]

Correct, it might be better the use the settings! I made changes to the BLEMIDI_ArduinoBLE.h that I have just uploaded - can you check if this works on your end?

Only the possible changes are related to h2zero/NimBLE-Arduino#473 (comment).
When we define what settings are necessary and what are their default values, I will add them to the code.

If NimBLE base lib has changed, it may a good moment to update this library (not only Client Mode) for use correctly struct settings in all hardware definitions.

[lathoub]

If NimBLE base lib has changed, it may a good moment to update this library (not only Client Mode) for use correctly struct settings in all hardware definitions.

+1, the MIDI library is also about to get a new release, so we can tag along shortly after

[RobertoHE]

@lathoub Please, check the autotest Logs of the project. CustomSettings.ino compiles successfully, but some errors appear in the other examples.
Please, use this branch (or my fork repository) to fix errors compilation in the examples.

In another way, Custom Setting using Struct is finished and tested.

[RobertoHE]

Check this post: arduino/arduino-cli#765 @lathoub

[RobertoHE]

DONE.
@lathoub consider extending Struct settings to other transport layers.

[lathoub]

Do you want to define LED-BUILTIN in the examples? (I believe this macro is defined in the device definition)

[RobertoHE]

Do you want to define LED-BUILTIN in the examples? (I believe this macro is defined in the device definition)

I am using PlatformIO within VisualStudio Code and this macro is not defined in my case, so I can't compile it. And I have had the same problem with the auto-test script.

The protection that I added in the examples is for defining this macro only in the case that is not previously declared. If you have this macro pre-declared, it will override nothing and will use the default value of LED-BUILTIN.

[lathoub]

I am using PlatformIO within VisualStudio Code and this macro is not defined in my case, so I can't compile it. And I have had the same problem with the auto-test script.

You got me to reconsider PlatformIO again :-) and ran into build-flags where you can set the LED_BUILTIN. Does that help you?

[env:esp wrover kit]
platform = espressif32
framework = arduino
board = esp-wrover-kit
monitor_speed = 115200
build_flags =
	; https://docs.espressif.com/projects/esp-idf/en/latest/get-started/get-started-wrover-kit.html#rgb-led
	-D LED_BUILTIN=2

[RobertoHE]

Yes, with build_flag you can set any #define in compiling step if compiler doesn't have this #define.

The problem (for a beginer with no knowledge about programming who only wants to run the example without editing anything) is that example codes don't compile, it is necessary to edit the code, adding #define LED_BUILTIN X (for arduino IDE) or build_flags = -D LED_BUILTIN=2 (for platformio). In other words, additional steps are necessary for running the example.

With those lines, LED_BUILTIN gets a default value in case it is not previously declared. If you set the value using #define or bulid_flags, the new value will overwrite the previous one. In all cases, the examples compile.

In my opinion, an example must compile and run autonomily without changes.