RSDK-9645 introduce consistent naming convention for frame system map objects #4664
Conversation
viambot
added
the
safe to test
|
Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!
|
| @@ -46,7 +46,7 @@ func (r *resultPromise) result() ([]node, error) { | |||
| return planReturn.steps, nil | |||
| } | |||
|
|
|||
| // linearizedFrameSystem wraps a framesystem, allowing conversion in a known order between a map[string][]inputs and a flat array of floats, | |||
| // linearizedFrameSystem wraps a framesystem, allowing conversion in a known order between a FrameConfiguratinos and a flat array of floats, | |||
There was a problem hiding this comment.
Should be FrameSystemInputs, right?
There was a problem hiding this comment.
Yeah thanks looks like the typo evaded my find and replace
referenceframe/input.go
Outdated
| // GetFrameInputs looks through the inputMap and returns a slice of Inputs corresponding to the given frame. | ||
| func GetFrameInputs(frame Frame, inputMap map[string][]Input) ([]Input, error) { | ||
| func (inputs FrameSystemInputs) GetFrameInputs(frame Frame) ([]Input, error) { |
There was a problem hiding this comment.
I wonder if functions like this should take a string rather than an entire frame?
There was a problem hiding this comment.
Good point there's no reason to pass the whole frame in this case
There was a problem hiding this comment.
Actually the whole function just becomes an index into a map and is used in just a couple places. I'm going to remove it
I agree. I was also toying with FrameSystemConfigurations but that's kind of adding another word into the already messy vernacular We can sit on this for a bit and try to think of something better if you want |
raybjork
changed the title
viambot
added
safe to test
We've talked about adding an object for
map[string][]Inputfor a while and I think now is the time since we have just added one formap[string]PoseInFrame. I wasn't sure what to call it since TrajectoryState would have been the correllary and sounds weird in the context of thereferenceframepackage. I opted for the explicitFrameSystemInputsand think that its clunky but clear. Then I went back and renamedPathStatestuff to be consistent with this. Hoping this change is for the better but if not we dont have to merge.