Feature/time sequencer python #156
Conversation
| if self.queue_size != 0 and len(self.messages) > self.queue_size: | ||
| del self.messages[0] |
There was a problem hiding this comment.
If I'm reading this correctly, I think that it should be documented that a queue_size == 0 means that there is effectively an unlimited queue.
src/message_filters/__init__.py
Outdated
| self.update_timer = self.node.create_timer( | ||
| self.update_rate.nanoseconds / 1e9, self.dispatch | ||
| ) |
There was a problem hiding this comment.
Since this spins a timer up, I think it would make sense to also have a corresponding shutdown function to clean up resources:
def shutdown(self):
self.update_timer.cancel()
self.node.destroy_timer(self.update_timer)
src/message_filters/__init__.py
Outdated
| delay = Duration(seconds=delay) | ||
| if not isinstance(update_rate, Duration): | ||
| update_rate = Duration(seconds=update_rate) | ||
| self.delay = delay |
There was a problem hiding this comment.
nit: add types to member variables
src/message_filters/__init__.py
Outdated
| ) | ||
| return None | ||
|
|
||
| def dispatch(self): |
There was a problem hiding this comment.
Consider prefixing this with a single _ to mark that it's for internal use only.
src/message_filters/__init__.py
Outdated
| raise RuntimeError("Already connected to an input filter.") | ||
| self.incoming_connection = input_filter.registerCallback(self.add) | ||
|
|
||
| def add(self, msg): |
There was a problem hiding this comment.
Also underscore here for internal function
|
Thanks @mjcarroll for reviewing, and apologies for the late response. I've made the changes as you suggested. Let me know if you would like me to address anything further. |
There was a problem hiding this comment.
There are some flake8 failures https://build.ros2.org/job/Rpr__message_filters__ubuntu_noble_amd64/116/
Add simplified python implementation for TimeSequencer filter.