PilzDE / pilz_application_templates

ROS application templates and example applications for pilz products.
Apache License 2.0
4 stars 4 forks source link

Introducing braketest & operation_mode support #8

Closed ct2034 closed 4 years ago

martiniil commented 5 years ago

@jschleicher I like your first suggested change more.

jschleicher commented 4 years ago

I've drawn the rename and PEP-8 out of this PR so this PR is just about braketest and opmode as its title suggests.

jschleicher commented 4 years ago

@martiniil @JonathanGruner I gave it another try rephrasing and grouping the options. I'm still unsure, what the sto option does? Is this used for braketest and operationmode as well? And why should a user disable operation mode support? I see just one usecase with safety function and one without - no need for finegrain feature selection.

martiniil commented 4 years ago

@martiniil @JonathanGruner I gave it another try rephrasing and grouping the options. I'm still unsure, what the sto option does? Is this used for braketest and operationmode as well? And why should a user disable operation mode support? I see just one usecase with safety function and one without - no need for finegrain feature selection.

@jschleicher Yes, I think the use-cases you desribe are sufficient. Currently, sto serves for specifying the safety controller, which is of course misleading.

edit: The braketest-module may be missing, but this could also be decided dependent on the name of the safety controller?

ct2034 commented 4 years ago

I would suggest the following: We merge this, as it is now. Because this will work with the current state of pilz_robots.

And then we continue the discussion about naming of sto there. There are more aspects to this: https://github.com/PilzDE/pilz_robots/pull/315 Ok, @martiniil @jschleicher ?

JonathanGruner commented 4 years ago

I read through your comments, and from mine/customer point of view I would write both tags below the tag with "sto" (which I requested to change to "device" because a STO is something else, see https://github.com/PilzDE/pilz_robots/pull/315 , and than maybe you can define your own device in the future), but commented.

But I would prefer to discuss how we design the user frontend together in the next sprint review on Wednesday.

ct2034 commented 4 years ago

remaining changes addressed in #16