Skip to content
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

APP-7497: Add Button and Switch APIs #619

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions proto/viam/component/button/v1/button.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
syntax = "proto3";

package viam.component.button.v1;

import "common/v1/common.proto";
import "google/api/annotations.proto";
import "google/protobuf/struct.proto";

option go_package = "go.viam.com/api/component/button/v1";
option java_package = "com.viam.component.button.v1";

// A ButtonService services buttons associated with a machine
service ButtonService {
// Pushes a button
rpc Push(PushRequest) returns (PushResponse) {
option (common.v1.safety_heartbeat_monitored) = true;
option (google.api.http) = {put: "/viam/api/v1/component/button/{name}/push"};
}

// 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"};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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"};

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy pasta

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}

message PushRequest {
string name = 1;
google.protobuf.Struct extra = 99;
}

message PushResponse {}

45 changes: 45 additions & 0 deletions proto/viam/component/switch/v1/switch.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
syntax = "proto3";

package viam.component.switch.v1;

import "common/v1/common.proto";
import "google/api/annotations.proto";
import "google/protobuf/struct.proto";

option go_package = "go.viam.com/api/component/switch/v1";
option java_package = "com.viam.component.switch.v1";

// A SwitchService services switches associated with a machine
service SwitchService {
stevebriskin marked this conversation as resolved.
Show resolved Hide resolved
// Turns on the switch
rpc On(OnRequest) returns (OnResponse) {
option (common.v1.safety_heartbeat_monitored) = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this heartbeat monitored? I think we're pushing to only do this for actuators.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

option (google.api.http) = {put: "/viam/api/v1/component/switch/{name}/on"};
}

// Turns off the switch
rpc Off(OffRequest) returns (OffResponse) {
option (common.v1.safety_heartbeat_monitored) = true;
option (google.api.http) = {put: "/viam/api/v1/component/switch/{name}/off"};
}

stevebriskin marked this conversation as resolved.
Show resolved Hide resolved
// 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"};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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"};

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catches, ty

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}

message OnRequest {
string name = 1;
google.protobuf.Struct extra = 99;
}

message OnResponse {}

message OffRequest {
string name = 1;
google.protobuf.Struct extra = 99;
}

message OffResponse {}

Loading