Keep Track of Active Notifiers. Make Notifier usable as ContextManager. #1890
Conversation
|
@zariiii9003 can we make this NotifierRegistry a little more "public"? For example we could have a Notifier factory (for a given Bus instance give me the existing Notifier or create a new one), etc. |
This would make things a bit more complicated.
However, a What kind of solution did you have in mind? |
This would be great! If I understand correctly, without such a classmethod this PR in its current state does not solve the example I put forth in #1888 (comment). A |
zariiii9003
force-pushed
the
notifier_registry
branch
from
e06fc55 to
e35610a
Compare
can/notifier.py
Outdated
| many listeners carry out flush operations to persist data. | ||
|
|
||
|
|
||
| :param bus: A :ref:`bus` or a list of buses to listen to. | ||
| :param bus: | ||
| A :ref:`bus` or a list of buses to listen to. |
There was a problem hiding this comment.
| A :ref:`bus` or a list of buses to listen to. | |
| A :ref:`bus` or a list of buses to consume messages from. |
can/notifier.py
Outdated
| self._lock = threading.Lock() | ||
|
|
||
| self._readers: List[Union[int, threading.Thread]] = [] | ||
| buses = self.bus if isinstance(self.bus, list) else [self.bus] | ||
| for each_bus in buses: | ||
| self._bus_list = self.bus if isinstance(self.bus, list) else [self.bus] |
There was a problem hiding this comment.
self._bus_list won't be correct after a user adds an additional Bus via add_bus, instead prefer to create an empty bus list here in the initializer, but add them inside the add_bus method.
There was a problem hiding this comment.
Good catch. The same was true for self.bus, i changed that to a property.
Notifier.stop(). Adapt examples to usewith can.Notifier(...)