Bluetooth for ESP32, nrf or not ?

After taking a closer look to latest changes in lib for bluetooth I have some questions.
I will start with setScan. As far as I can see, I have to create an event with a JsVar for adv. This is taken in callback and converted to a BluetothDevice object.

1. BluetoothDevice is ifdef with nrf, how would I bypass this, or should I return a simple object ?
   
2. first an object is created, which is next converted to a BluetoothDevice, why not create a BluetoothDevice directly ?
   setScan is defined as nrf.setScan. This is the way how all the descriptions are done. IMHO it would be a kind of confusing for users of other boards to use a nrf command. I could imagine to add one more wrapper which connects the same function to Bluetooth Object. Same for setServices etc.
   
3. would this option add too much overhead
   
4. is there any other reason not to do it ?
   BluetoothUtils holds a lot of helpful functions, which could be used for other boards as well. As far as I understand, there are also some nrf only functions in.
   
5. could we split this into 2 files, BluetoothUtil and nrfUtil for example ?
   
6. or ifdef the nrf relevant function ?
   

BluetoothDevice is ifdef with nrf, how would I bypass this, or should I return a simple object ?

I'd maybe change the ifdef to be BLUETOOTH instead?

first an object is created, which is next converted to a BluetoothDevice, why not create a BluetoothDevice directly ?

You mean here: https://github.com/espruino/Espruino/blob/master/targets/nrf5x/bluetooth.c#L615

and then:
https://github.com/espruino/Espruino/blob/master/libs/bluetooth/jswrap_bluetooth.c#L1258

It's just because I'm trying to do the bare minimum inside the IRQ. So the IRQ (first one) only allocates the data it needs - then outside of an IRQ it does more allocation.

In your case it works well, because you should be able to use the existing code in jswrap_bluetooth as-is?

setScan is defined as nrf.setScan. This is the way how all the descriptions are done. IMHO it would be a kind of confusing for users of other boards to use a nrf command. I could imagine to add one more wrapper which connects the same function to Bluetooth Object. Same for setServices etc.

Yes, I know. I think eventually I'd change NRF to be BLE or Bluetooth and would then make NRF==BLE on nRF5x devices to be compatible.

would this option add too much overhead

Not if done right - I wouldn't add 'shadow' functions though - that would use up a bit more memory for no real reason.

is there any other reason not to do it ?

Not really - but for the moment I'd leave it as NRF so as not to confuse your porting efforts - and then at some point soon I can change that in main Espruino branch.

If it really irks you, perhaps you could just execute the code var BLE=NRF at boot time on ESP32 for now?

BluetoothUtils holds a lot of helpful functions, which could be used for other boards as well. As far as I understand, there are also some nrf only functions in. could we split this into 2 files, BluetoothUtil and nrfUtil for example ? or ifdef the nrf relevant function ?

Are you up to date with GitHub? I have now moved BluetoothUtils outside of the nrf5x directory: https://github.com/espruino/Espruino/tree/master/libs/bluetooth

nrfUtil is basically what bluetooth.c should be? Board-specific bluetooth functions.

Currently bluetooth_utils.c does have some ifdefs, but they're in what should hopefully be quite generic functions like bleVarToUUID - hopefully you can just ifdef ESP32 in there as well for the small parts that are specific to each device.