bpo-36590: Add Bluetooth RFCOMM support for Windows. #12767
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for your contribution, we look forward to reviewing it! |
|
I believe unit tests are certainly needed for such a change. |
Modules/socketmodule.c
Outdated
| @@ -518,6 +518,15 @@ remove_unusable_flags(PyObject *m) | |||
| #endif | |||
| #endif | |||
|
|
|||
| #if defined(MS_WINDOWS) && defined(AF_BTH) | |||
There was a problem hiding this comment.
Looks like AF_BTH is defined in all supported versions of Windows, so no need for the second part of the test.
There was a problem hiding this comment.
Done -- only checks for MS_WINDOWS now.
Modules/socketmodule.c
Outdated
| @@ -1877,6 +1916,7 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, | |||
| case AF_BLUETOOTH: | |||
| { | |||
| switch (s->sock_proto) { | |||
| #ifndef MS_WINDOWS | |||
There was a problem hiding this comment.
Can we test whether macros are defined rather than checking for platform? (Also, is BTHPROTO_L2CAP the same as BTPROTO_L2CAP here?)
There was a problem hiding this comment.
I changed all of the ifndef MS_WINDOWS tests to ifdef BTPROTO_X and added/changed such tests in other cases (e.g. ifdef BTPROTO_SCO instead of checking for not FreeBSD).
BTHPROTO_L2CAP is the same as BTPROTO_L2CAP. Windows defines the constant, but winsock does not implement L2CAP support. I was able to build with this, but creating a socket with BTPROTO_L2CAP with winsock fails.
|
Added some basic unittests that check for some constants and try to create sockets with various protocols. I did not add any tests for the address parsing/rendering as I wasn't sure if there is a good way to do so. The only way to hit |
The change was pretty straightforward. The only difficulty was that Windows' SOCKADDR_BTH contains member names incompatible with the _BT_RC_MEMB() macros. To workaround this, a new struct, SOCKADDR_BTH_REDEF was defined and contains unions with both the Windows and Linux member names. Support for RFCOMM, L2CAP, HCI, SCO is based on the BTPROTO_* macros being defined. Winsock only supports RFCOMM, even though it has a BTHPROTO_L2CAP macro. L2CAP support would build on windows, but not necessarily work. This also adds some basic unittests for constants (all of which existed prior to this commit, just not on windows) and creating sockets. pair: Nate Duarte <slacknate@gmail.com>
|
LGTM. Sorry for the delay in getting it merged |
) Support for RFCOMM, L2CAP, HCI, SCO is based on the BTPROTO_* macros being defined. Winsock only supports RFCOMM, even though it has a BTHPROTO_L2CAP macro. L2CAP support would build on windows, but not necessarily work. This also adds some basic unittests for constants (all of which existed prior to this commit, just not on windows) and creating sockets. pair: Nate Duarte <slacknate@gmail.com>
) Support for RFCOMM, L2CAP, HCI, SCO is based on the BTPROTO_* macros being defined. Winsock only supports RFCOMM, even though it has a BTHPROTO_L2CAP macro. L2CAP support would build on windows, but not necessarily work. This also adds some basic unittests for constants (all of which existed prior to this commit, just not on windows) and creating sockets. pair: Nate Duarte <slacknate@gmail.com>
) Support for RFCOMM, L2CAP, HCI, SCO is based on the BTPROTO_* macros being defined. Winsock only supports RFCOMM, even though it has a BTHPROTO_L2CAP macro. L2CAP support would build on windows, but not necessarily work. This also adds some basic unittests for constants (all of which existed prior to this commit, just not on windows) and creating sockets. pair: Nate Duarte <slacknate@gmail.com>
The change was pretty straightforward. The only difficulty was that
Windows' SOCKADDR_BTH contains member names incompatible with the
_BT_RC_MEMB() macros. To workaround this, a new struct,
SOCKADDR_BTH_REDEF was defined and contains unions with both the Windows
and Linux member names.
pair: Nate Duarte slacknate@gmail.com
https://bugs.python.org/issue36590