-
Notifications
You must be signed in to change notification settings - Fork 91
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
Analog domain #211
Analog domain #211
Conversation
Note: This will be breaking ABI, but not API. |
I am not sure it this is the right way. I would prefer to guide people setting this up on the teach pendant. Though they need to remember to save the installation |
I don't quite understand why that should be the case. We offer a functionality to set an analog output and this PR merely adds the possibility to specify whether this is a current or a voltage. Is what you are saying that users should select on the TP which domain to use and the functionality in the client library shouldn't modify the domain at all? We could also add a third state "don't change" which could be the default. One problem there would be that at least for ROS 1 the default value would be Thinking about it, that might be the best solution: Provide a default value of |
I like the idea of giving the flexibility to do both. |
b19b26f
to
1d8c366
Compare
I've updated the PR accordingly. When not passing the domain, it is not set in the RTDEWriter. |
This way it is more explicit what is actually happening.
05bb5f6
to
e32e642
Compare
This adds the possibility to specify the output domain when setting an analog output.
Tests are still missing, hence the draft status.
Edit: Tests are now there.