Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
9 changes: 9 additions & 0 deletions conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ def generate(self):
tc.cache_variables["VIAMCPPSDK_OFFLINE_PROTO_GENERATION"] = self.options.offline_proto_generation
tc.cache_variables["VIAMCPPSDK_USE_DYNAMIC_PROTOS"] = True

# We don't want to constrain these for conan builds because we
# don't know the context where we might be being built. We
# should permit the build if it works. Also, even the C++ SDK
# is warnings clean on the modern compilers we use in CI, it,
# or headers from its dependencies, might throw warnings with
# older compilers, and we should still allow a build there.
tc.cache_variables["VIAMCPPSDK_ENFORCE_COMPILER_MINIMA"] = False
tc.cache_variables["VIAMCPPSDK_USE_WALL_WERROR"] = False

tc.cache_variables["VIAMCPPSDK_BUILD_TESTS"] = False
tc.cache_variables["VIAMCPPSDK_BUILD_EXAMPLES"] = False

Expand Down
4 changes: 0 additions & 4 deletions src/viam/sdk/services/motion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ bool operator==(const obstacle_detector& lhs, const obstacle_detector& rhs) {
return lhs.vision_service == rhs.vision_service && lhs.camera == rhs.camera;
}

bool operator==(const Motion::steps& lhs, const Motion::steps& rhs) {
return lhs.steps == rhs.steps;
}

bool operator==(const Motion::plan_status& lhs, const Motion::plan_status& rhs) {
return std::tie(lhs.reason, lhs.state, lhs.timestamp) ==
std::tie(rhs.reason, rhs.state, rhs.timestamp);
Expand Down
18 changes: 6 additions & 12 deletions src/viam/sdk/services/motion.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,19 +113,13 @@ class Motion : public Service {
friend bool operator==(const plan_status_with_id& lhs, const plan_status_with_id& rhs);
};

/// @struct steps
/// @brief An individual "step", representing the state each component (keyed as a fully
/// qualified component name) should reach while executing that step of the plan.
using step = std::unordered_map<std::string, pose>;

/// @brief An ordered list of plan steps.
/// @ingroup Motion
struct steps {
/// @brief An individual "step", representing the state each component (keyed as a fully
/// qualified component name) should reach while executing that step of the plan.
typedef std::unordered_map<std::string, pose> step;

/// @brief The ordered list of steps.
std::vector<step> steps;

friend bool operator==(const struct steps& lhs, const struct steps& rhs);
};
using steps = std::vector<step>;

/// @struct plan
/// @brief Describes a motion plan.
Expand All @@ -142,7 +136,7 @@ class Motion : public Service {
std::string execution_id;

/// @brief An ordered list of plan steps.
struct steps steps;
steps steps;
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like this is still causing issues, now with additional compilers. a couple thoughts. so i think usually our alias declarations would use camel case, and if the declarations above were using Step = ... and using Steps = ... there would be no issue here.

that said, i'm sort of thinking we should just write std::vector<[Ss]tep> and call it a day. the alias for unordered_map feels ergonomically justified, a vector alias less so. and generally speaking we don't have many instances in the sdk of doing using meows = std::vector<meow> and i suppose i'm not crazy about the practice in general.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that makes sense. I've made that change.


friend bool operator==(const plan& lhs, const plan& rhs);
};
Expand Down
7 changes: 3 additions & 4 deletions src/viam/sdk/services/private/motion_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,17 +139,16 @@ Motion::plan_status_with_id from_proto(const service::motion::v1::PlanStatusWith

Motion::steps steps_from_proto(
const google::protobuf::RepeatedPtrField<service::motion::v1::PlanStep>& proto) {
using step = Motion::steps::step;
std::vector<step> steps;
Motion::steps steps;
for (const auto& ps : proto) {
step step;
Motion::step step;
for (const auto& component : ps.step()) {
step.emplace(component.first, from_proto(component.second.pose()));
}
steps.push_back(std::move(step));
}

return {steps};
return steps;
}

Motion::plan plan_from_proto(const service::motion::v1::Plan& proto) {
Expand Down
4 changes: 2 additions & 2 deletions src/viam/sdk/services/private/motion_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ service::motion::v1::PlanStatus to_proto(const Motion::plan_status& ps) {
return proto;
}

service::motion::v1::PlanStep to_proto(const Motion::steps::step& step) {
service::motion::v1::PlanStep to_proto(const Motion::step& step) {
service::motion::v1::PlanStep proto;
for (const auto& kv : step) {
service::motion::v1::ComponentState cs;
Expand All @@ -67,7 +67,7 @@ service::motion::v1::Plan to_proto(const Motion::plan& plan) {
*proto.mutable_id() = plan.id;
*proto.mutable_component_name() = to_proto(plan.component_name);
*proto.mutable_execution_id() = plan.execution_id;
for (const auto& step : plan.steps.steps) {
for (const auto& step : plan.steps) {
*proto.mutable_steps()->Add() = to_proto(step);
}

Expand Down
Loading