Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Master mode, Pin and SSP #3219

Merged
merged 44 commits into from Oct 1, 2019

Conversation

@IonicEV
Copy link
Contributor

commented Sep 17, 2019

I witnessed a lot of folks struggling with master mode on ESP32 (initiating connection to other slave devices). Some examples were available(example_spp_initiator_demo.c), but they lacked support for IO, congestions and other aspects covered nicely in original BluetoothSerial class. I took a step forward and combined code provided in example_spp_initiator_demo.c and BluetoothSerial class to enable Master mode, Pin and SSP for legacy BT in BluetoothSerial class. Hopefully, you would find it useful.

@IonicEV

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

Looks like build #2408.4 errored out due to inability to access main repo, please advise.

fatal: unable to access 'https://github.com/espressif/arduino-esp32.git/': Failed to connect to github.com port 443: Connection timed out

@IonicEV

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

All checks are passed, what is the next step, please forgive me - it is first time for me on github.

@luc-github

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2019

Just wait - review will be done with some feedback ^_^

@me-no-dev

This comment has been minimized.

Copy link
Member

commented Sep 17, 2019

Very nice :) this is really useful!
But, some of the techniques, excessive logging, too many static variables, constants, not using proper API to wait for events. It all makes the code a bit hard to read and not so efficient to execute. How about I give you a hand and touch up a bit your additions?

@IonicEV

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

Sure, let me push the latest iteration that uses resolved address as preference on connect() and get rid of redundant _remote_address, and more of technique in example. all done on my side. I see [Event Group API] is used for signaling in existing code.

…e now redundant _remote_address
@IonicEV

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

Pushed pin logic re-work

@IonicEV

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

I made some effort to address some of the issues you noted. I was not sure you have time to work on it, as you could be quite busy. Hopefully, you need to spend less time refactoring the code.

Victor Tchistiak added 13 commits Sep 17, 2019
Victor Tchistiak
… for disconnect event. + timeout for disconnect()
…s during scan in info mode as it is very difficult to find out sometimes. Fixed get_name_from_eir() not resetting the name when called.
Victor Tchistiak
@IonicEV

This comment has been minimized.

Copy link
Contributor Author

commented Sep 18, 2019

At this point my only concerns are with delegation to user to set timeouts to keep it from any unexpected crash due to concurrency. I timed the ops and have pretty good idea how to set timeouts in connect() and disconnect() operations appropriately, so the result code would reflect the true state, not just async API call return value.

Victor Tchistiak added 2 commits Sep 18, 2019
…currency and more authoritative status returned back to caller. Automatic disconnect in connect() methods
Victor Tchistiak
Victor Tchistiak and others added 10 commits Sep 18, 2019
Victor Tchistiak
Victor Tchistiak
Victor Tchistiak
@luc-github

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2019

@IonicEV is PR can be tested now ? or do you plan to do new update ?
I am interrested in testing it ^_^

@IonicEV

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2019

@IonicEV is PR can be tested now ? or do you plan to do new update ?
I am interrested in testing it ^_^

Sure, I am not planning on any changes on my side. I am curious to see if "client" mode pin is working as expected (when ESP32 request pin for new connection). The doc not explicitly mentions the use of this pin methods to enforce incoming connections, mostly about outgoing one.

@luc-github

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2019

@IonicEV I have moved your code to my latest esp32-arduino git directory and launched
SerialToSerialBTM.ino
I have paired my phone without any pin, I have uncommented the line SerialBT.setPin(pin); but I still can pair without any pin request
and I always get after pairing : Failed to connect. Make sure remote device is available and in range, then restart app.
Do you have a test case to share ? Do I test in wrong way ?

@IonicEV

This comment has been minimized.

Copy link
Contributor Author

commented Sep 26, 2019

As I mentioned earlier I did not find in documentation suggestion those functions (esp_bt_gap_set_pin & esp_bt_gap_pin_reply) are working for incoming connections (your use case), only outgoing connections were mentioned and those are working for outgoing connections. For incoming connections to use those functions, as you mentioned, it needs some kind of library module compiled with specific options or containing the code to utilize esp_bt_gap_set_pin function for incoming connections. This does not preclude people from using pins for outgoing connections that is supported currently by the firmware.

@IonicEV

This comment has been minimized.

Copy link
Contributor Author

commented Sep 26, 2019

Do you have a test case to share ? Do I test in wrong way ?

No, your use case has nothing to do with master mode, you are connecting to the ESP32(slave), not from ESP32(master). "I have paired my phone without any pin" - means you phone is the master and your ESP32 is the slave. If you want to test Master scenario - connect to BT OBDII adapter(always a slave device with SPP service) from ESP32(master). In order to connect in master mode, one has to know target device name or BT address and pin if it is enforced by slave device and different from default pin. BTW, BluetoothSerial class supports SPP only.

me-no-dev and others added 4 commits Oct 1, 2019
Victor Tchistiak
@me-no-dev me-no-dev merged commit 38c4c06 into espressif:master Oct 1, 2019
21 checks passed
21 checks passed
Arduino 0 on ubuntu-latest
Details
Arduino 1 on ubuntu-latest
Details
Arduino 2 on ubuntu-latest
Details
Arduino 3 on ubuntu-latest
Details
Arduino 4 on ubuntu-latest
Details
Arduino 5 on ubuntu-latest
Details
Arduino 6 on ubuntu-latest
Details
Arduino 7 on ubuntu-latest
Details
Arduino 8 on ubuntu-latest
Details
Arduino 9 on ubuntu-latest
Details
Arduino 10 on ubuntu-latest
Details
Arduino 11 on ubuntu-latest
Details
Arduino 12 on ubuntu-latest
Details
Arduino 13 on ubuntu-latest
Details
Arduino 14 on ubuntu-latest
Details
Arduino on windows-latest
Details
Arduino on macOS-latest
Details
PlatformIO on ubuntu-latest
Details
PlatformIO on windows-latest
Details
PlatformIO on macOS-latest
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@me-no-dev

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

alright :) let's give it a world

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.