ausocean / cloud

GNU General Public License v3.0
1 stars 1 forks source link

Model: NewDevice() func #203

Open ao-david opened 3 days ago

ao-david commented 3 days ago

This change adds a NewDevice method which will accept function options (DeviceOption) to generate a new device using defaults.

This implementation relies heavily on the use of maps which should hopefully allow these values to be more loosely coupled from the implementation, allowing for easier and more clear modification, however, these maps could be replaced by the switch statement if that was preferable.

depends on #202

Starts addressing issue #195

ao-david commented 2 days ago

All comments addressed. Now using WithInputs, WithOutputs, WithVariables, WithSensors, WithActuators

saxon-milton commented 2 days ago

I'm wondering if we should reconsider the design of this slightly. It feels like we're conflating the concept of a Device, with a system comprising the Device, as well as the Sensors and Actuators that connect up to the Device's inputs and outputs.

In other words, does it really make sense that our NewDevice constructor takes a bunch of options where we provide the Actuators and Sensors, and then the constructor spits out only a Device ? Also, we shouldn't need to have WithInputs options as well as WithSensors options right ? Shouldn't we be able to derive the inputs from the sensors; if that's exactly the reason that they're linked conceptually in the first place ?

Perhaps we need a new struct like ControllerSystem, which has the Device, Sensor and Actuator fields e.g.

type ControllerSystem struct {
    device Device // maybe even []Device ?
    sensors []SensorsV2
    actuators []ActuatorsV2
}

(not to be used as a datastore entity necessarily). We'd then have a NewControllerSystem constructor that can take functional options, similarly to how we've done it, i.e. WithActuators, WithSensors etc

This would then populate the ControllerSystem struct as appropriate.

Also, I'm not sure about how we've got the datastore operations built into the options and constructor, this seems inflexible/lacks modularity. I think we should have a couple of adapter functions instead; one for putting a ControllerSystem into the datastore (puts the individual entities into the datastore i.e. Device, Actuators etc) and on for getting a ControllerSystem (gets the required entities from the datastore and constructs a ControllerSystem).

We can use all this to create some factories for creating default controller systems for example

func rigControllerSystem() *ControllerSystem {
    return NewControllerSystem(
                    WithActuators(defaultRigActuators()),
                    WithSensors(defaultRigSensors()),
                    WithVariables(defaultRigVariables()),
             ),
}

func jettyRigControllerSystem() *ControllerSystem {
    // blah
}

func reefTreatmentControllerSystem() *ControllerSystem {
    /// blah
}

I think this could be a really powerful way to start abstracting our systems.