olliw42 / mLRS

2.4 GHz & 915/868 MHz & 433 MHz/70 cm LoRa based telemetry and radio link for remote controlled vehicles
GNU General Public License v3.0
279 stars 58 forks source link

Adds Bluetooth connectivity option to mLRS Bridge. #115

Closed rotorman closed 11 months ago

rotorman commented 11 months ago

This PR changes mLRS WiFi bridge to mLRS WiFi & Bluetooth bridge -> thus a new name mLRS Wireless Bridge.

With define WIRELESS_PROTOCOL either WiFi TCP (=0), WiFi UDP (=1) or Bluetooth (=2) can be selected.

Quick test with an Espressif ESP32-DevKitC V4 gave promising results. Would be sweet to get some further tests with other hw before merging.

Update: longer test with M5Stack ATOM Lite shows that this works nicely.

olliw42 commented 11 months ago

very nice I just happen to also work on this

I think the code is unneccesarily complicated by being pedantic on WIFI and BT. I think WIFI_nBLUETOOTH should go, and instead BT could be just WIFI_PROTOCOL = 2. The configure section has become quite difficult IMHO, and also the code below. Similarly the files do IMHO not need to be renamed ... I mean, just consider what's going to happen when ESPNow or BLE or what the heck else would be added ... and we also would have to adapt the docs all time ... looks just like a predictable nightmare to me :)

I was thinking about WIRELESS instead of WIFI, but I really see no big problem with staying with WIFI/wifi.

pl consider it protocol 2

just a style thing, #if's I don't consider code and code thus isn't indented to follow the #if levels ... usually (sometimes it can make it look ugly)

rotorman commented 11 months ago

I actually started with protocol 2, but did not like this as Bluetooth is NOT a WiFi protocol.

But we could come up with another name, maybe PROTOCOL_TYPE and this being 0 for WiFi TCP, 1 for WiFi UDP and 2 for Bluetooth. How does this sound?

The indentation of #if's IMO really helps the readability.

olliw42 commented 11 months ago

I understand the BT is not wifi argument, but as said, it IMHO introduces lots of complexity and make-it-difficult-for-users for what is really just semantics. I think the decission is simply, do we want to demonstrate that we understand that BT is not wifi or do we want new users with potentially low code skills enter.

the problem I have with things like PROTOCOL_TYPE is that they really say nothiong, I mean, is this MAVLink v1 vs v2? Or LTM vs transparent?

The indentation of #if's IMO really helps the readability.

I will see when I see the whole thing :)

rotorman commented 11 months ago

Plz see last commit. If you are not cool with PROTOCOL_TYPE then how about WIRELESS_PROTOCOL?

rotorman commented 11 months ago

setPin does not seem to work for Bluetooth device in client mode. At least I tried various combinations, and got never asked for PIN to connect to. If you have an idea, I am open to hear it.

What I did was:

#define USE_BLUETOOTH_PIN
const char *bluetoothPin = "1234";

and

SerialBT.begin(bluetooth_device_name);
#if defined(USE_BLUETOOTH_PIN)
  SerialBT.setPin(bluetoothPin);
#endif

with no luck.

olliw42 commented 11 months ago

setPin does not seem to work for Bluetooth device in client mode.

I haven't yet tried my self, but what you tried looks identical to what common tutotials say to do ... sad. I have no better idea unfortunately.

olliw42 commented 11 months ago

this really lgtm now. just few very tiny things

olliw42 commented 11 months ago

I'm trying this code now and have a number of issues.

1) It took me few minutes to realize that teh BluetoothSerial library is apparanetly not available for many boards, but just few. I actually installed it and they it complained it finds two installs LOL. Anyway, when merged I will do some changes to give the user a better chance here.

2) Much more problematic, the serial traffic seems to work only in one direction, namely the direction tx -> MisssionPlanner, but not MissionPlanner -> tx. Btw, I have tested it also with TCP and UDP, and both worked fine, so it's not setup mistake like bad wire or so.

That is, I do see all the telemetry messages (HUD etc. pp) in MissionPlanner, but e.g. param upload just doesn't work. This suggests that something doesn't work with SerialBT.available()/SerialBT.read() functions. To see if it's the code changes I put upon you in this section I reverted to what you had initially with the same sympthom.
The LED switches to slow blinking indicating "connected". That is, SerialBT.available() must be at least some times positive.

Any idea what I may do wrong? EDIT: got it. BUG :)

WORKS

very nice very nice !!!

olliw42 commented 11 months ago

many thx sir, this is very cool!

brad112358 commented 11 months ago

I had been thinking of doing this same thing because of the same issue as people were mentioning on discord: I can't seem to get my phone to connect to the internet via Cellular while the WiFi is connected to the esp32, even though the phone knows the WiFi network has no internet connectivity. I think this worked back in my crossfire days. I wonder if the esp32 AP code is advertising a default route. We don't want that as it might cause the phone to route everything to the WiFi even when the cellular network is available.

Anyway, I decided to test this PR and it works great with the R9M and the M5Stack M5Stamp Pico. Very nice! I don't see any lost messages in most connection sessions. Sometimes just a few messages are lost when first connecting.

Unfortunately, the ESP32C3 chip doesn't support BT SPP, only BT LE.