RSDK-10936 introduce RobotFrameSystem interface

[raybjork]

This PR implements the changes needed to implement viamrobotics/api#713 . As a part of this the FrameSystemService interface is changing so that it 1:1 matches the available methods on the robot proto. The intention here is that this is a zero output change so if you see something that looks like it goes beyond a refactor it is probably wrong

I recommend starting this review by looking at robot.go and framesystem.go. The changes should make sense after reading these

[github-actions]

Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!

component	function
base	IsMoving
board	GPIOPinByName
camera	Properties
encoder	Properties
motor	IsMoving
sensor	Readings
servo	Position
arm	EndPosition
audio	MediaProperties
gantry	Lengths
gripper	IsMoving
input_controller	Controls
movement_sensor	LinearAcceleration
power_sensor	Power
pose_tracker	Poses
motion	GetPose
vision	GetProperties

[biotinker]

Is there a reason not to just have this be a resource?

[raybjork]

I'm not sure. Going to defer to @cheukt on this

[cheukt]

let's make this a resource too

[raybjork]

On closer inspection this is not possible. By adding theResource interface we give the FrameSystemService a Reconfigure method, which is also present in the Robot interface, so this causes problems.

Is there a reason I'm not seeing you would like to make this a resource?

[cheukt]

it should be a resource because we'll be treating it like a resource in order for modular framesystem to work. but it should be ok as long as the underlying frameSystemService struct implements those methods, which it seems to do. we can hash out anything that doesn't work when we try to implement a modular framesystem

[biotinker]

Another way to solve the problem would be, for robot to not also be a frame system service.

The reason for doing so I assume is for it to be a zero output change and not break a bunch of existing robot clients?

[raybjork]

Well the methods are on the robot client itself, I think it does make sense for them to continue to be on the interface

[raybjork]

framesystem.Service is the resource, this is just the interface defining the functions that are common to both this and the robot

[biotinker]

I guess, what's the final form of this? How will a module get at the framesystem?

I'm assuming we're going to split the API and make a FrameSystemClient?

When that happens, that then would probably be the time to make it a resource. Can we add a TODO?

[raybjork]

Yeah this is work that Netcode is going to pick up after I get this initial change in. I'm not too sure on the details of how they will do it tbh. I don't think a TODO is necessary for work that is imminently scheduled

[biotinker]

This feels more difficult to use than what existed before, especially absent some helper that actually returns the list of resources.

[raybjork]

You aren't wrong. CurrentInputs is not an API method though and I want there to be a 1:1 correspondence between API and interface here. If you have a better suggestion I'm open to it

[biotinker]

I think this needs to be in the API for the FS service.

The motivation behind the creation of this service, is "so the framesystem can be used on modules". What we have here is indeed a service that will provide a FrameSystem object to a module, but as a module cannot at present actually create an enumeration of resources on the robot without a Robot client, in terms of actual utility we're just running in place here.

The framesystem is useless without the inputs with which to call it. The inputs cannot be created without a robot client, or else manually making sure that the module calling this has a dependency on every single other robot component (don't add a new one or rename an old one and forget to update the dependency list!)

[raybjork]

I disagree. Inputs are not a concept in proto (yet) and I want to avoid biting that off. I don't think its that bad to have this be a helper function. At the end of the day anybody who has a resource client can call the associated InputEnabled methods on the resource anyway. All this was doing was providing a weird backdoor where you could somehow know about resources that you might not even have a dependency on in the first place. (i.e you write a motion service that doesnt depend on every arm (this isn't a strawman, this was useful for the DC demo) you would get back information about something that should be unknown to you)

[biotinker]

I mean, by returning the whole framesystem, we're already telling resources about other resources they don't directly depend on.

By requiring CurrentInputs to take a list of resources, we're basically saying that in order to know where things are, you need to be able to control them. That is, in order to get read access, you must have write access.

We're creating a situation here where a module using a framesystem, goes from "impossible", to "difficult, tedious, fragile and error-prone". The difference between "difficult, tedious, fragile and error-prone" and "easy" here is the creation of the data structure used as input to the frame system.

Calling CurrentInputs should IMO not be massively more difficult than calling NewFromService. I don't think the dis-inclusion of Inputs in proto until now is a huge problem, since this would be something specific to robots/frame system services, not something that components would have to concern themselves with.

[biotinker]

Oh; in that case, how is a module expected to access it? Supplying the module with the robot's own API key etc is intended to remain the "correct" way of doing things?

[raybjork]

I'm not completely sure about that. Something having to do with it being passed by default to module constructors. Netcode will own this part of the work and @cheukt can weigh in here

[cheukt]

the module will be passed the framesystem resources by default in the dependencies map

[biotinker]

the module will be passed the framesystem resources

To clarify; you're saying that the framesystem resource will be passed to all modules by default, or that a module receiving a framesystem will automatically be passed all resources in the frame system?

[cheukt]

the former - the framesystem resource will be passed to all modules by default

[cheukt]

generally looks fine to me

[cheukt]

let's make this a resource too

[biotinker]

LGTM