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.

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

RSDK-2692 - Add collectors for MovementSensor LinearAcceleration and Orientation #2258

Merged
merged 1 commit into from
Apr 24, 2023
Merged

RSDK-2692 - Add collectors for MovementSensor LinearAcceleration and Orientation #2258

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions components/movementsensor/movementsensor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ func init() {
h, err := ms.CompassHeading(ctx, make(map[string]interface{}))
return Heading{Heading: h}, err
})
registerCollector("LinearAcceleration", func(ctx context.Context, ms MovementSensor) (interface{}, error) {
v, err := ms.LinearAcceleration(ctx, make(map[string]interface{}))
return v, err
})
registerCollector("Orientation", func(ctx context.Context, ms MovementSensor) (interface{}, error) {
v, err := ms.Orientation(ctx, make(map[string]interface{}))
Copy link
Contributor

Choose a reason for hiding this comment

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

Since Orientation returns an interface, and these are persisted by converting them to a proto struct using this function, which requires the input to be a struct, this unfortunately won't work. This output will need to wrapped in/converted to some concrete type. The proto defined OrientationResponse type here should work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This did seem to work. Orientation output looked like this:
{"Imag":0,"Jmag":0,"Kmag":0,"MetadataIndex":3,"Real":1,"TimeReceived":{"seconds":1682100838,"nanos":271460000},"TimeRequested":{"seconds":1682100838,"nanos":271455000}}

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting - I must be mis-remembering. Looks like the concrete type it's using is a quat.Number then. I would still recommend wrapping in that proto type so that this is consistent (should be an easy mapping using the orientations .AxisAngles(), which seems to be the representation the proto interface uses)

Copy link
Contributor

Choose a reason for hiding this comment

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

Though actually I see Readings uses this quat value too. I think this is definitely fine for now, but as a team we should revisit the pattern here soon (put time on Thursday to discuss)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the explanation, and sounds good!

return v, err
})
}

// SubtypeName is a constant that identifies the component resource subtype string "movement_sensor".
Expand Down