OpenCyphal / pycyphal

Python implementation of the Cyphal protocol stack.
https://pycyphal.readthedocs.io/
MIT License
117 stars 105 forks source link

Migrate to Nunavut v2 #318

Closed Willmac16 closed 5 months ago

Willmac16 commented 6 months ago

Addresses #277

I'm still a bit unsure about how I'm handling nunavut_support.py, so I'd appreciate feedback on that and anything else.

coveralls commented 6 months ago

Coverage Status

coverage: 94.802% (-0.4%) from 95.246% when pulling b8f2e16fb35f90ae98d2a78c9ab0a96fe9673208 on Willmac16:Willmac16/issue277 into a6a899cc35dac20a313f231f3b17289004a84679 on OpenCyphal:master.

Willmac16 commented 6 months ago

It looks like Yakut needs the pycyphal.dsdl.([a-z_]+) -> nunavut_support.$1 treatment.

Willmac16 commented 6 months ago

Unrelated, should the package be marked as zip_safe in setup.cfg at this point?

pavel-kirienko commented 6 months ago

Unrelated, should the package be marked as zip_safe in setup.cfg at this point?

Maybe, but it would probably be a micro-optimization and this flag is obsolete either way, so I prefer not to touch it. See https://setuptools.pypa.io/en/latest/deprecated/zip_safe.html

pavel-kirienko commented 6 months ago

It looks like Yakut needs the pycyphal.dsdl.([a-z_]+) -> nunavut_support.$1 treatment.

I have opened a ticket about this in the Yakut repo. Would you be available to submit the required changes there? For testing, you can replace the PyCyphal dependency specification as follows:

 install_requires =
-    pycyphal[transport-udp,transport-serial,transport-can-pythoncan] ~= 1.8
+    pycyphal[transport-udp,transport-serial,transport-can-pythoncan] ~= git+https://github.com/Willmac16/pycyphal@Willmac16/issue277

You can temporarily replace the Yakut dependency in PyCyphal integration tests in a similar fashion.

Willmac16 commented 6 months ago

Once everything in this PR is good to go 🤞

~Can I squash my commits into one to keep the commit history of upstream clean.~

Edit: Looks like squashed merges are a built in GitHub feature so I'll refrain from doing it manually.