MovementSensor #143
Closed
MovementSensor #143
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zaporter-work
force-pushed
the
zp/movementsensor
branch
from
09102ce to
ddee5d0
Compare
zaporter-work
commented
Aug 28, 2023
| // uses). | ||
| struct orientation { | ||
| double o_x, o_y, o_z, theta; | ||
| // TODO(pre-merge) move the to/from protos in here. Nicer that way |
There was a problem hiding this comment.
Maybe? I saw you do this for some other methods? I kept the current way with everything in the component..
Not sure what the plan is
zaporter-work
commented
Aug 28, 2023
| std::unordered_map<std::string, float> result = movementsensor->get_accuracy(extra); | ||
| response->mutable_accuracy()->empty(); | ||
| for (const auto& i : result) { | ||
| response->mutable_accuracy()->emplace(i.first, i.second); |
There was a problem hiding this comment.
Strangely this like works on my laptop but does not in CI. Different proto versions maybe?
/tmp/src/viam/sdk/components/movementsensor/server.cpp:216:39: error: no member named 'emplace' in 'google::protobuf::Map<std::basic_string<char>, float>' [clang-diagnostic-error]
response->mutable_accuracy()->emplace(i.first, i.second);
There was a problem hiding this comment.
Yes, almost certainly different proto version. I remember encountering this same issue when working on the MLModel stuff and had to stop using emplace.
|
Implemented in #171 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Howdy!
I needed MovementSensor bindings for a personal project so I created them.
I don't have time to polish this up and bring it through a PR, but I thought I would share the code in case someone wanted to take it from here. If everyone is busy, that is fine too! Just thought I should share.
There are a few things to figure out (and the linter is complaining about a few missing
consts), but the general structure is there and the tests pass on my machine...Weirdest part of this PR is that the file is named
movementsensor/movementsensor.hpp.Pros of
movementsensor.hpp:movementsensor.gomovementsensor.protomovementsensor, notmovement_sensorCons of
movementsensor.hpp:movement_sensor.hppwould be easier to read and would make more sensemovement_sensor.pymovement-sensor.ts