-
Notifications
You must be signed in to change notification settings - Fork 174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CapnProto Publisher and Subscriber to Python API #1089
base: master
Are you sure you want to change the base?
Conversation
Thank you @CruxDevStuff for contributing ! We will check this next week. If you have python samples for sending and receiving capnproto messages like those ones in cpp, it would be great too. |
I've added an Example |
Great. Thank you. |
@CruxDevStuff can you please create an eclipse account, then we can merger your PR after the review has been done. |
Sorry for the delay, I've created an Eclipse account that's linked my Github(I can view this PR from my eclipse account page). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, sorry for the long wait.
Thanks for making this contribution, however I do have a few remarks:
- Dependencies: we now have a dependency to
pycapnp
, it needs to be added to requirements.txt for ecal. - Before running the samples, the user will have to run the capnproto compiler on the schema file themselves. I'd prefer if we'd check in the generated code here as we do in the protobuf case. Or, alternatively, have a way to generate this on python import? At least it should be in the README.md
@@ -131,9 +131,116 @@ def _on_receive(self, topic_name, msg, time): | |||
proto_message.ParseFromString(msg) | |||
self.callback(topic_name, proto_message, time) | |||
|
|||
class ProtoSubscriber(MessageSubscriber): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are now two ProtoSubscriber classes. Can you please remove one of them?
@@ -0,0 +1,8 @@ | |||
@0xb6249c5df1f7e512; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you use the same message like is used for the c++ samples? this way we can communicate the samples with python send / c++ receive and vice versa.
Ah and one more thing, the samples should be placed in a subfolder |
Pull request type
Please check the type of change your PR introduces:
What is the new behavior?
Does this introduce a breaking change?