Feature: Synchronized bus instance #315
Conversation
|
TODO: Documentation. Done. |
doc/bus.rst
Outdated
| @@ -37,10 +39,24 @@ Receiving | |||
| ''''''''' | |||
|
|
|||
| Reading from the bus is achieved by either calling the :meth:`~can.BusABC.recv` method or | |||
| by directly iterating over the bus:: | |||
| by directly iterating over the bus: | |||
There was a problem hiding this comment.
This is not a mistake, see here.
doc/bus.rst
Outdated
|
|
||
| It can be used exactly like the normal :class:`~can.Bus` class: | ||
|
|
||
| my_bus = can.Bus(interface='socketcan', channel='vcan0') |
There was a problem hiding this comment.
Shouldn't this be can.ThreadSafeBus?
setup.py
Outdated
| @@ -56,6 +57,7 @@ | |||
| python_requires=">=2.7,!=3.0,!=3.1,!=3.2,!=3.3", | |||
| install_requires=[ | |||
| 'setuptools', | |||
| 'wrapt ~= 1.10', | |||
| ] + (['subprocess32 ~= 3.2.7'] if version_info.major < 3 else []), | |||
There was a problem hiding this comment.
Unrelated to this pull request, but why is setuptools here? The whole setup.py file requires setuptools to be installed already. And is subprocess32 really needed? Python 2.7 has subprocess.check_output or does it not work as expected? In any case it should be moved to extras_require as it is only needed for socketcan.
There was a problem hiding this comment.
The setuptools package is added for completeness, it is not exactly wrong that it is an dependency. It doesn't really help with anything though. Shall we remove it or leave it? I think it doesn't really matter.
Hm, I agree that subprocess32 should be moved into extras_require. But I added it as an dependency because of this note at the top of the documentation you linked:
POSIX users (Linux, BSD, etc.) are strongly encouraged to install and use the much more recent subprocess32 module instead of the version included with python 2.7. It is a drop in replacement with better behavior in many situations.
I simply followed that advice.
There was a problem hiding this comment.
setuptools is not a dependency for the can module itself. It is setuptools that handles the the install_requires at setup time so it is a bit strange and I haven't seen any other packages do the same.
Have you tested if it works on Python 2.7 using the built-in subprocess module? If it does then it seems unnecessary to add an extra dependency. After all, it is a pretty simple function that is not critical.
There was a problem hiding this comment.
Okay, I'll remove the setuptools dependency.
Lets's see what Travis CI will tell us about the built-in subprocess module ...
|
Okay, so it was possible to remove the |
|
Do you know why |
|
It must contain “test” in the filename. |
doc/bus.rst
Outdated
| --------------- | ||
|
|
||
| This thread safe version of the bus class can be used by multiple threads at once. | ||
| Sending and receiving is locked seperatly to avoid unnessesary delays. |
There was a problem hiding this comment.
I'd mention that a call from one thread will block until the bus is free - rather than an async approach using a threadsafe queue.
Now it works. Thanks! @hardbyte |
|
Shall I merge it? |
|
|
Adds a
can.ThreadSafeBusthat can be used as a thread safe drop-in replacement forcan.Bus. The send and receive methods are synchronized separately.