Classic BT SPP enhancement (IDFGH-2275)

[BeckerMarkus]

Evaluating Classic BT SPP in acceptor mode, I found issues to connect to the acceptor on the ESP32 using different clients.

While I got a connection from Android and Windows, I could not get a connection using "rfcomm connect" on Ubuntu.

Further, I found it hard to establish concurrent connections.
I placed a question on the esp32.com forum that kept being unanswered, while there are other users that seem to experience similar sounding problems.

After some research I found:
esp_spp_start_srv( ESP_SPP_SEC_NONE,ESP_SPP_ROLE_SLAVE, 0, "SPP_SERVER")
will make the server choose a free SCN (server channel number), which is 2 on startup but will change for subsequent (concurrent) connections. The connecting client is required to use the servers SCN for the connection to be established.

Android and (my) Windows client, seem to silently try out different SCN's until the connection request is accepted by the ESP32 stack, while on Linux the channel must be known and set as a parameter of "rfcomm connect".

Reading the sources, I found a hint in port_api.c/ RFCOMM_CreateConnection:
...
** Server can call this function with the same scn parameter multiple times if
** it is ready to accept multiple simultaneous connections.
**
** DLCI for the connection is (scn * 2 + 1) if client originates connection or
** existing none initiator multiplexer channel. Otherwise it is (scn * 2).
** For the server DLCI can be changed later if client will be calling it using
** (scn * 2 + 1) dlci.
...

I found no way to configure the stack in a way, that the DLCI update on the server side takes place with the existing code, while its perfectly possible that I missed something fundamental.

The proposed changes work for me:

• if esp_spp_start_srv() is called with ESP_SPP_ANY_SCN (0) as SCN the server will connect regardless of the clients SCN
• on connection (ESP_SPP_SRV_OPEN_EVT) the server will report the used SCN to the app (which might use this to select a "service")
• no change in the esp_spp_api is needed

The introduced changes are heavy and might break nearly everything else, not only RFCOMM/SPP and I'm quite sure that some more work should be done, before merging this pull request.

I'm not sure how important the proposed function is to others, or if I've missed an important point, so please have a look and comment.

Best,
Markus

[CLAassistant]

All committers have signed the CLA.

[BeckerMarkus]

While trying to reduce Heap memory usage, I found a bug that leads to an error, if more than around MTU bytes data is in buffer. The bug affects master, not only this pull request, so the missing pointer operation should be merged to master in all cases.
Further, I think the fixed size allocated buffers in PORT_WriteDataCO() are in many cases to large (4096+16Bytes) and somewhat not flexible. I found no reason why we should not allocate just the needed size there.

I added a commit to fix these issues.

Best,
Markus

[projectgus]

Hi @BeckerMarkus ,

Thanks for sending these fixes and your patience waiting for a response.

I've reminded some of the Bluetooth team about this PR and they should review it shortly.

Angus

[blueMoodBHD]

Hi @BeckerMarkus About your problem, maybe we can solve without any changes.

1. About the problem of Linux connected SPP failed:
   We can find the SCN first (esp_spp_start_discovery() is used to do this ), then connect to the remote Bluetooth device on the specified channel

2. About concurrent connection
   You can call esp_spp_start_srv() twice or more, to create more than one servers. Then esp_spp_start_discovery() will find more than one SCN. We can establish two or more connections between two devices.

Can those solve your problem? If those can, I think, enable the server to accept any client connection regardless of clients SCN is not the best solution.

[BeckerMarkus]

Hi @blueMoodBHD,

discovering the SCN first can be a solution, if both endpoints reside on ESP32 devices, or at least if the initiator side can be configured to work that way.
That might be out of reach when an app uses a serial port emulation on windows or a rfcomm device on linux for example.
In my case the ESP32 replaces an older design and existent clients must be supported.

Setting up multiple servers (calling esp_spp_start_srv() multiple times) seems redundant to me, as the first instance can and will handle multiple connections anyway.
I'm a bit worried about resources too, while I did not try to do that by now.

As mentioned, I'm not sure how helpful the change is to others, the lack of comments up to now make me think not many are interested.

Would you please have a look into the changed PORT_WriteDataCO (commit 0bbf700). I think that could help regardless of the other stuff.

Best
Markus

[blueMoodBHD]

Hi @BeckerMarkus ,
About the change of PORT_WriteDataCO (commit 0bbf700). There are no problem either the changed memory allocation strategy or the bugfix.
Thanks for these fixes.

[BeckerMarkus]

Hi @projectgus,

thanks for merging the bug fix.

I feel a bit uncertain about how to proceed from here. While the proposed change works good for me and might help others because it somewhat simplyfies SPP usage (makes the acceptor act more like the user might expect when passing 0 as SCN to esp_spp_start_srv()).

On the other side, the rfcomm/spp part has enough of complexity in it from my point of view and it might not be the best idea to add more tricks to it :) - so I completely understand blueMoodBHDs position.

So it might be best to close this PR now?

Markus

[projectgus]

Hi @BeckerMarkus ,

I'm probably not the best person to talk to about this, @blueMoodBHD is a member of the Bluetooth team so he understands the details much better than me.

To check I do understand correctly:

• You're replacing a legacy SPP "server" (acceptor) device that would previously accept connections on any SCN.
• The client(s) are multiple Windows/Linux devices. But only one connects at a time.
• The Linux clients choose an arbitrary (random?) SCN when they initiate a connection via the RFCOMM layer, so you can't know this at the time you are calling esp_spp_start_srv()?
• Windows is less of a problem because the Windows driver will retry different SCN values until one is accepted.

Is that right?

Setting up multiple servers (calling esp_spp_start_srv() multiple times) seems redundant to me, as the first instance can and will handle multiple connections anyway.
I'm a bit worried about resources too, while I did not try to do that by now.

If it's possible to know the range of SCNs clients will use, I think this seems like a good option. I believe the additional memory usage of each additional SPP slot is pretty small, and this gives you predictable usage if multiple clients connect concurrently.

[blueMoodBHD]

HIi @BeckerMarkus , @projectgus ,
We can find the SCNs servers used before connecting. Such as sdptool browse .

[BeckerMarkus]

Hi @projectgus, @blueMoodBHD,

thanks for looking into this again, sorry, I was not clear enough in my last comment.

It was more about the right way of dealing with a PR under the given circumstances.

As it looks like, the bug fix has been merged, the other part won't be merged and I very well can accept that. But I still think, the changes could be useful, may be to a smaller number of systems i.e. more special cases.

So, in that case, should it be me closing the PR or would Angus do that or would it be best to leave it open for a while, just in case someone comes around?

Looking at Angus comment makes me think I did not manage to explain the issue in an understandable way, again, sorry for that.

Let me retry here, using a few more words just for reference:

The situation

• To establish a SPP connection, the initiator needs to know the acceptors Bluetooth address and an SCN the server accepts.
• On the ESP32 the acceptor can be configured to use a fixed SCN (for example 4). Then the initiator can connect by using the acceptors Bluetooth address and set SCN to 4
• Or, on the ESP32 the acceptor can be configured to use a free SCN by passing 0 as SCN value. Then the ESP32 acceptor uses a SCN he finds to be available. The initiator then needs to find out
  that SCN value, before he can establish the connection. He can do so by using the Bluetooth discovery mechanism.

The problem

• Historically I had a Bluetooth SPP to RS232 module, connected to a serial port. I never had to deal with the SCN - left the modules parameters default
  and the (Android, Linux, Windows) clients connected without problems.
• Then, with the ESP32 the initiators did not connect as they did against the old design.

The proposed solution

• I did some research and found out that a possible design for an SPP acceptor is, that it accepts connections from initiators requesting a connection using a SCN freely chosen by the initiator.
• It might be possible to achieve this with the existing sources by calling esp_start_srv multiple times with SCN values 2,4,6,8,.. but that would require a fair amount of connection handles, security contexts,...
• I ended up with the proposed solution that (if 0 is passed as SCN to esp_start_srv()) establishes a listening connection on a free channel and then updates the channel according to an initiators request.

While that works great for me, the downside is obvious - it introduces more complexity (and maybe bugs, compatibility issues, ...) to an already non trivial construct.

I think I made a misjudgement about how useful that change is to others too.

Systems that use a SPP acceptor on the ESP32 together with initiators that can not be easily configured to discover the acceptors SCN (like a Bluetooth SPP/RS232 Module, Windows virtual com port,...) can make profit from the change, I thought there should be many of those.

In real world, SPP on the ESP32 might not be used that often in such a context because most systems might use BLE now or the initiator side can be easy made to comply with the ESP32 SPP implementation.

Markus

[pilatomic]

Hi @BeckerMarkus,
I just encountered the same issue, which greatly impacts the usability of the ESP32 in products where BT SPP connection to a PC is required.
Thank you for posting this patch, I am a bit disappointed that it did not get merged to ESP-IDF, since this is far from being just an edge case.
Edit : also I can confirm this is as much as a problem on Windows ( tested on Win7 x64 pro) than on Linux (Mint 19.2 x64)

Regards