bpo-30987 - Support for ISO-TP protocol in SocketCAN #2956
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. Thanks again to your contribution and we look forward to looking at it! |
pylessard
changed the title
|
After second thoughts, only the |
|
Pull request is re-open. All dependencies over the third party project have been removed. |
Modules/socketmodule.c
Outdated
| @@ -6993,6 +7055,8 @@ PyInit__socket(void) | |||
| PyModule_AddIntMacro(m, CAN_SFF_MASK); | |||
| PyModule_AddIntMacro(m, CAN_EFF_MASK); | |||
| PyModule_AddIntMacro(m, CAN_ERR_MASK); | |||
|
|
|||
| PyModule_AddIntMacro(m, CAN_ISOTP); | |||
Modules/socketmodule.c
Outdated
| @@ -1913,6 +1927,54 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, | |||
| Py_DECREF(interfaceName); | |||
| return 1; | |||
| } | |||
| #endif | |||
|
|
|||
| #if defined(CAN_ISOTP) | |||
Modules/socketmodule.c
Outdated
| @@ -1869,7 +1882,9 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args, | |||
| } | |||
| #endif | |||
|
|
|||
| #if defined(AF_CAN) && defined(CAN_RAW) && defined(CAN_BCM) | |||
| #if defined(AF_CAN) | |||
There was a problem hiding this comment.
Please use ifdef for simple checks: #ifdef AF_CAN
Lib/test/test_socket.py
Outdated
| def testBind(self): | ||
| try: | ||
| with socket.socket(socket.PF_CAN, socket.SOCK_DGRAM, socket.CAN_ISOTP) as s: | ||
| addr = (self.interface,0x123,0x456) |
There was a problem hiding this comment.
Style nit pick: addr = self.interface, 0x123, 0x456
Lib/test/test_socket.py
Outdated
| # most systems limit IFNAMSIZ to 16, take 1024 to be sure | ||
| with socket.socket(socket.PF_CAN, socket.SOCK_DGRAM, socket.CAN_ISOTP) as s: | ||
| self.assertRaisesRegex(OSError, 'interface name too long', | ||
| s.bind, ('x' * 1024,1,2)) |
There was a problem hiding this comment.
Please use
with self.assertRaisesRegex(OSError, 'interface name too long'):
s.bind(('x' * 1024, 1, 2))
to make it more readable.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I didn't expect the Spanish Inquisition! |
|
Nobody expects the Spanish Inquisition! @tiran: please review the changes made to this pull request. |
|
Your PR is still missing a space after comma in |
|
@tiran Changes done sir! |
* Added support for CAN_ISOTP protocol * Added unit tests for CAN ISOTP * Updated documentation for ISO-TP protocol * Removed trailing whitespace in documentation * Added blurb NEWS.d file * updated Misc/ACKS * Fixed broken unit test that was using isotp const outside of skippable section * Removed dependecy over third party project * Added implementation for getsockname + unit tests * Missing newline at end of ACKS file * Accidentally inserted a type in ACKS file * Followed tiran changes review #1 recommendations * Added spaces after comma
This Pull Request add the support of CAN ISO-TP address extensions in the socket module by enhancing
getsockaddrargandgetsockname.Note to core developpers : I believe this pull request should be backported back to Python 3.3 where SocketCAN support have been added.
https://bugs.python.org/issue30987