APP-7497: Add Button and Switch APIs #619
Conversation
|
|
||
| // DoCommand sends/receives arbitrary commands | ||
| rpc DoCommand(common.v1.DoCommandRequest) returns (common.v1.DoCommandResponse) { | ||
| option (google.api.http) = {post: "/viam/api/v1/component/gripper/{name}/do_command"}; |
There was a problem hiding this comment.
| option (google.api.http) = {post: "/viam/api/v1/component/gripper/{name}/do_command"}; | |
| option (google.api.http) = {post: "/viam/api/v1/component/switch/{name}/do_command"}; |
There was a problem hiding this comment.
Great catches, ty
|
|
||
| // DoCommand sends/receives arbitrary commands | ||
| rpc DoCommand(common.v1.DoCommandRequest) returns (common.v1.DoCommandResponse) { | ||
| option (google.api.http) = {post: "/viam/api/v1/component/gripper/{name}/do_command"}; |
There was a problem hiding this comment.
| option (google.api.http) = {post: "/viam/api/v1/component/gripper/{name}/do_command"}; | |
| option (google.api.http) = {post: "/viam/api/v1/component/button/{name}/do_command"}; |
| service SwitchService { | ||
| // Turns on the switch | ||
| rpc On(OnRequest) returns (OnResponse) { | ||
| option (common.v1.safety_heartbeat_monitored) = true; |
There was a problem hiding this comment.
why is this heartbeat monitored? I think we're pushing to only do this for actuators.
There was a problem hiding this comment.
why is this heartbeat monitored? I think we're pushing to only do this for actuators.
I originally commented on this as well and deleted the comment. Maybe a button press can trigger some long-running task that needs to be stopped if the session drops. Not sure how that'll actually work without a Stop function...figured this isn't worth a discussion now as there's no harm in this.
There was a problem hiding this comment.
It seems like this is analogous to board's SetGPIO API, which isn't session monitored, and, actually, DoCommand itself. will it break anything if we want to remove it in the future? If so, let's just remove it now.
There was a problem hiding this comment.
I'm happy to remove this for now - I had the same thought when copy-pasting gripper. It's easy enough to add in the future and there's not yet clear value.
| string name = 1; | ||
| uint32 position = 2; |
There was a problem hiding this comment.
hmm, do we want to allow negatives too?
There was a problem hiding this comment.
I don't see a reason to - position should just be a non-negative index representing the possible positions. The only case I can think of is if we wanted a negative position to be an error state, but I don't see that being useful. In error cases, the handlers should return errors.
There was a problem hiding this comment.
I'm thinking of the most common toggle switch to me - the power drill direction - where that switch makes sense to me as a -1, 0, 1 switch. But meh 🤷🏼♀️
| rpc SetPosition(SetPositionRequest) returns (SetPositionResponse) { | ||
| option (google.api.http) = {put: "/viam/api/v1/component/switch/{name}/set_position"}; | ||
| } | ||
|
|
||
| // Get the position of the switch | ||
| rpc GetPosition(GetPositionRequest) returns (GetPositionResponse) { | ||
| option (google.api.http) = {put: "/viam/api/v1/component/switch/{name}/get_position"}; | ||
| } |
There was a problem hiding this comment.
I don't love these API names, since it's more ambiguous than ToggleTo and ToggleActive but I don't care about this API enough to argue this, and I hope fleet enjoys owning their two new component APIs 😈
There was a problem hiding this comment.
Like we chatted about, I'm happy to change the names but don't feel strongly. Will leave as-is.
There was a problem hiding this comment.
@stevebriskin we'll defer to you here.
There was a problem hiding this comment.
IMO, all names are good enough and not worth debating in the interest of speed.
This adds the APIs for two new components: Button and Switch. Request from Eliot.
Next:
Review requests