[AR-6566] object tag feature #58
Conversation
custom.proto
Outdated
| @@ -21,6 +21,11 @@ message ProfilingResultSet { | |||
| map<string, sensr_proto.ProfilingResult> algo_nodes = 2; | |||
| } | |||
|
|
|||
| enum Stage { | |||
| STAGE_NONE = 0; | |||
| STAGE_DRIVING = 1; | |||
There was a problem hiding this comment.
@minhup could you add more stages?
There was a problem hiding this comment.
For now, those two are enough. One for being controlled, one for not. We can add more later.
There was a problem hiding this comment.
STAGE_NONE -> NONE and STAGE_DRIVING -> CONTROLLED
There was a problem hiding this comment.
Unfortunately we should keep the STAGE_ prefix because enum in protobuf is plain enum so NONE could conflict with other enums. Also, the style guide suggests putting the type name as a prefix - ref.
enum FooBar {
FOO_BAR_UNSPECIFIED = 0;
FOO_BAR_FIRST_VALUE = 1;
FOO_BAR_SECOND_VALUE = 2;
}Regarding CONTROLLED, I also considered the name but wasn't sure if the other project would also "control" something like moving a trailer or controlling a crane to pick up a container. What do you think?
There was a problem hiding this comment.
Controlled meant being controlled by motion planner. We only send FoG for controlled objects. I didn't consider the trailer cases yet since we are not doing the project yet so no need to support it now. But we need the controlled stage for building our full stack (perception + control). For the prefix, it's fine to keep it though if it makes teh conflict.
There was a problem hiding this comment.
The car is being driving doesn't necessary being controlled by motion planner.
There was a problem hiding this comment.
True, that's a good point. I'll rename it with STAGE_CONTROLLED.
|
The commit 1a004b2 from this pull request is the current version that we use for argos while this PR is not merged. This should be avoided. Please proceed with the PR |
|
Thanks for the reminder. I completely forgot the PR. Let me merge it ASAP. |
|
@sebastian-seoul-robotics can you check the PR? I believe it doesn't break the backward-compatibility as the stage is newly introduced. |
Stageenum to custom.proto