Arduino-BLE-MIDI icon indicating copy to clipboard operation
Arduino-BLE-MIDI copied to clipboard

BleClient Custom settings

Open RobertoHE opened this issue 3 years ago • 14 comments

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

RobertoHE avatar Jun 24 '22 20:06 RobertoHE

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

lathoub avatar Jun 27 '22 09:06 lathoub

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

If you change this, the example compiles with errors?

RobertoHE avatar Jun 27 '22 10:06 RobertoHE

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?

lathoub avatar Jun 27 '22 11:06 lathoub

Client example code modified.

RobertoHE avatar Jun 27 '22 11:06 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.

RobertoHE avatar Nov 08 '22 07:11 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?

lathoub avatar Nov 08 '22 08:11 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?

Only the possible changes are related to https://github.com/h2zero/NimBLE-Arduino/issues/473#issuecomment-1306785035. 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.

RobertoHE avatar Nov 08 '22 08:11 RobertoHE

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

lathoub avatar Nov 08 '22 09:11 lathoub

@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 avatar Nov 28 '22 07:11 RobertoHE

Check this post: https://github.com/arduino/arduino-cli/issues/765 @lathoub

RobertoHE avatar Nov 28 '22 08:11 RobertoHE

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

RobertoHE avatar Nov 28 '22 16:11 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.

RobertoHE avatar Dec 12 '22 16:12 RobertoHE

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

lathoub avatar Jan 08 '23 10:01 lathoub

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.

RobertoHE avatar Jan 08 '23 14:01 RobertoHE