-
-
Notifications
You must be signed in to change notification settings - Fork 105
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
Extend the application-layer API to avoid hard-coded protocol-related constants in applications #154
Conversation
I was trying to work through this and found a bug with the DSDL generation:
I get this when running |
It was fixed about one month ago: OpenCyphal/pydsdl#61 Please make sure you are running the latest PyDSDL: |
Okay with updated dependancies (python package management is just a joy...) I got everything to run well, except for orchestration but I'm assuming that's not important to this PR, and don't have too much time to play with this. One nit is that the demo app's filename should probably be specified immediately prior to the code, the snippet given in "Running the Application" is the first reference to the filename used for the demo. |
The directory removal problem should be fixed now but you are right, this is not that important. Do you think such register integration can be implemented in uavcan.rs? |
Yep, although I don't plan on implementing it where I am currently working (which is roughly equivalent to the presentation layer in pyuavcan). I want the core A user-friendly "application" crate on top of the core crate would likely be what I would want to work on after I hit 1.0, and register integration is definitely a natural feature to add there. |
Okay. I think it's critical though to make the application crate no-std as well. We don't want embedded developers to reinvent this wheel. |
|
It is supposed to do that when you run the application for the first time (i.e., the database file is not yet created) without exporting environment variables. Can you confirm that it happens after you export the variables?
Kind of like this one? https://github.com/pavel-kirienko/yukon/blob/master/test.orc.yaml In a way, that's what the last part of the tutorial is about. |
I'd comment out the line that requires ncat and change the comment to "if you have ncat you can uncomment this line and..." (.penv) $ yakut orc launch.orc.yaml
/bin/sh: 1: ncat: not found |
Yeah. The ports and the IP coming from a config file would make this demo so much better. |
…nitialization on Windows
@thirtytwobits check out my latest changes. I think I addressed everything except:
This is not really meant to be very human-friendly. I think it makes sense to start with the basics even if they are not particularly convenient and then gradually introduce more advanced matters like the orchestrator. The orchestration file is essentially the config file you are looking for, but if we introduce it from the start it may get confusing.
I addressed this by making the demo easier to follow along step-by-step such that by the time the reader gets to the orchestrator the file should already be created. I also added another hint about this right after the orchestration script for good measure.
Not reproducible, neither locally nor in CI. |
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.
SO much better now! Thanks Pavel.
…odule is not used by the library itself, it is needed only for testing the demo)
This is an implementation and a demonstration in support of OpenCyphal/public_regulated_data_types#109.
Instead of specifying the port-ID explicitly when instantiating a port:
node.presentation.make_publisher(Type, 1234)
Now, one uses indirection to source the port-ID from the registry:
node.make_publisher(Type, 'my_subject')
The new node factory
make_node()
removes boilerplate and hard-coded identifiers.Please find the documentation for this change on the readthedocs PR build site:
It would be super if one could follow the tutorial along to make sure that it doesn't omit any crucial details. If you do that, note that you will need to install Yakut from a branch for now (I will merge/release it immediately after this PR is accepted):
I do not recommend reviewing the code because our limited resources are better spent on more important things.