Skip to content

RSDK-6829 - Add machine part id to ResourceName message #467

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 1, 2024
Merged

RSDK-6829 - Add machine part id to ResourceName message #467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2091aa6
change
cheukt Mar 8, 2024
1f19da9
robot -> machine
cheukt Mar 29, 2024
211cca3
Merge branch 'main' into robot-part-id
cheukt Mar 29, 2024
a828f61
test
cheukt Mar 29, 2024
8e6cd53
Built new protos from a828f61
github-actions[bot] Mar 29, 2024
211e60b
Merge branch 'main' into robot-part-id
cheukt Apr 1, 2024
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
477 changes: 245 additions & 232 deletions common/v1/common.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions component/encoder/v1/encoder.pb.gw.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions component/gantry/v1/gantry.pb.gw.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions component/gripper/v1/gripper.pb.gw.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions component/movementsensor/v1/movementsensor.pb.gw.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions component/sensor/v1/sensor.pb.gw.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions gen/js/common/v1/common_pb.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ export class ResourceName extends jspb.Message {
getName(): string;
setName(value: string): void;

hasMachinePartId(): boolean;
clearMachinePartId(): void;
getMachinePartId(): string;
setMachinePartId(value: string): void;

serializeBinary(): Uint8Array;
toObject(includeInstance?: boolean): ResourceName.AsObject;
static toObject(includeInstance: boolean, msg: ResourceName): ResourceName.AsObject;
Expand All @@ -35,6 +40,7 @@ export namespace ResourceName {
type: string,
subtype: string,
name: string,
machinePartId: string,
}
}

Expand Down
50 changes: 49 additions & 1 deletion gen/js/common/v1/common_pb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,8 @@ proto.viam.common.v1.ResourceName.toObject = function(includeInstance, msg) {
namespace: jspb.Message.getFieldWithDefault(msg, 1, ""),
type: jspb.Message.getFieldWithDefault(msg, 2, ""),
subtype: jspb.Message.getFieldWithDefault(msg, 3, ""),
name: jspb.Message.getFieldWithDefault(msg, 4, "")
name: jspb.Message.getFieldWithDefault(msg, 4, ""),
machinePartId: jspb.Message.getFieldWithDefault(msg, 5, "")
};

if (includeInstance) {
Expand Down Expand Up @@ -750,6 +751,10 @@ proto.viam.common.v1.ResourceName.deserializeBinaryFromReader = function(msg, re
var value = /** @type {string} */ (reader.readString());
msg.setName(value);
break;
case 5:
var value = /** @type {string} */ (reader.readString());
msg.setMachinePartId(value);
break;
default:
reader.skipField();
break;
Expand Down Expand Up @@ -807,6 +812,13 @@ proto.viam.common.v1.ResourceName.serializeBinaryToWriter = function(message, wr
f
);
}
f = /** @type {string} */ (jspb.Message.getField(message, 5));
if (f != null) {
writer.writeString(
5,
f
);
}
};


Expand Down Expand Up @@ -882,6 +894,42 @@ proto.viam.common.v1.ResourceName.prototype.setName = function(value) {
};


/**
* optional string machine_part_id = 5;
* @return {string}
*/
proto.viam.common.v1.ResourceName.prototype.getMachinePartId = function() {
return /** @type {string} */ (jspb.Message.getFieldWithDefault(this, 5, ""));
};


/**
* @param {string} value
* @return {!proto.viam.common.v1.ResourceName} returns this
*/
proto.viam.common.v1.ResourceName.prototype.setMachinePartId = function(value) {
return jspb.Message.setField(this, 5, value);
};


/**
* Clears the field making it undefined.
* @return {!proto.viam.common.v1.ResourceName} returns this
*/
proto.viam.common.v1.ResourceName.prototype.clearMachinePartId = function() {
return jspb.Message.setField(this, 5, undefined);
};


/**
* Returns whether this field is set.
* @return {boolean}
*/
proto.viam.common.v1.ResourceName.prototype.hasMachinePartId = function() {
return jspb.Message.getField(this, 5) != null;
};





Expand Down
1 change: 1 addition & 0 deletions proto/viam/common/v1/common.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ message ResourceName {
string type = 2;
string subtype = 3;
string name = 4;
optional string machine_part_id = 5;
Copy link
Contributor

Choose a reason for hiding this comment

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

why optional?

Copy link
Member Author

Choose a reason for hiding this comment

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

everything else in the resource name should always be filled out, so using optional here to be a bit more clear about the optionality. It doesn't seem like we are super consistent about this though, so could see making it a non-optional field as well

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, that was gonna be my next question - it doesn't seem like we are consistent with using the optional keyword vs relying on implicit optionality in protos (e.g. this field will just be a blank string)

Copy link
Contributor

Choose a reason for hiding this comment

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

i don't have have a strong opinion on this, so won't block - I'm okay making this explicitly optional

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'd like to start using optional a bit more, I think it makes it easier to tell what the response shape is like by just reading the proto

Copy link
Member

Choose a reason for hiding this comment

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

Do we not also want the machine ID? Or just the part ID?

Copy link
Member Author

Choose a reason for hiding this comment

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

just part id for now, that's what was scoped and most likely to be useful - user can get cloud metadata if they need more of that stuff

}

message BoardStatus {
Expand Down
6 changes: 3 additions & 3 deletions service/slam/v1/slam.pb.gw.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.