WHOIGit / PhytO-ARM

PhytO-ARM: Phytoplankton Observing for Automated Real-time Management
https://hablab.whoi.edu/phyto-arm/
GNU General Public License v3.0
0 stars 0 forks source link

AML CTD config #6

Closed mbrosnahan closed 1 year ago

mbrosnahan commented 1 year ago

CTD messages associate different messages by port, e.g.: /ctd/aml/port7/par

How will current code handle swap between AML CTDs that may be configured with a different array of sensors or installed in different locations?

If this needs to be coded, can we add fields to the configuration field that map ports when an AML ctd is used? There are similar differences between RBR and SeaBird data streams.

rgov commented 1 year ago

The profiler node takes a configuration parameter of what topic it is applying peak picking to. If your AML is configured differently you can just edit this parameter to use a different topic name.

ROS also allows topics names to be remapped so in the general case this would just be a change to the launch file.

mbrosnahan commented 1 year ago

Easier/better to track through mission specific config files. Is there a simple way to set and reference config variables in the launch file?

rgov commented 1 year ago

What is a "mission specific config file"? If it is different from a regular config file, the concept doesn't exist yet and needs to be defined.

The config YAML file already contains:

profiler:
    resolution: 0.02  # m
    data_topic: "/ctd/aml/port3/chloroblue"
    data_field: "value"

Can't you just update this? It can also be changed dynamically at runtime with the rosparam tool, as the README shows:

https://github.com/WHOIGit/PhytO-ARM/blob/d3941a80c915b55ace1e2b444e4e3980b7e496e2/README.md?plain=1#L34

Launch files are kind of limited but in the YAML I also have:

launch_args:
    ctd: aml
    ...

This launch_args section contains variables that the launch file can use. The phyto-arm script takes these variables and passes them so the launch file can use them:

https://github.com/WHOIGit/PhytO-ARM/blob/d3941a80c915b55ace1e2b444e4e3980b7e496e2/src/phyto_arm/launch/phyto_arm.launch#L11-L16

If you wanted to, you could add a remapping to the CTD node in the launch file like:

<node name="ctd" ...>
    <remap from="/ctd/aml/port7/par" to="$(arg my_topic_setting)" />
</node>

but I don't think I understand why this is preferable.


For the AML node specifically, the output of the node tells us which port the various sensors are attached to. The RBR doesn't provide this.

mbrosnahan commented 1 year ago

My understanding is that params defined under 'profiler:' control profile speed and which parameter is used for peak finding/IFCB sample depth choice.

Any AML unit may be reconfigured as needed for a particular deployment. So, port7 might be used for PAR in one deployment, FDOM in another, chloroblue, etc. It'd be valuable to define, e.g.:

launch_args: ctd: aml port1: cond port2: temp_ct port3: turbidity port4: phycoerythrin

RBR units aren't reconfigurable in the same way but do vary from one instrument to another. So port1 on SN:X won't necessarily be the same as port1 on SN:Y instrument.

Pushing this detail to config file is helpful because it these are an obvious starting point for setting up or updating a system. We would likely save configs with filenames that are specific to a given deployment (e.g., seatrac_15Apr2023.yaml). This will be helpful for tracking what we've set up at different time and for identifying most helpful starting point when we set up a new system/create another config.

rgov commented 1 year ago

But the AML already tells you what port1 is assigned to, why would you want to configure it and add an opportunity for human error?

The profiler resolution field is for the resample rate of the signal, not speed.

You are free to add whatever extra data or comments you want to the config file; it won't affect anything unless the code uses a given variable.

mbrosnahan commented 1 year ago

But the AML already tells you what port1 is assigned to, why would you want to configure it and add an opportunity for human error?

Agree with this. Issue was to flag this as a concern. Have we tested to confirm that it's 'auto configuring' correctly? It'd be easy to test with system on bench right now. Some configuration tracking will also be provided if config file can be updated to serve 'static' info through web_node.

rgov commented 1 year ago

Have we tested to confirm that it's 'auto configuring' correctly?

The proof is in the fact that the topic names are getting set up at all. It comes across the wire like port2[data=Pressure,0.071390,dbar][rawi=ADC,844470,2sComp] and gets translated to /ctd/aml/port2/pressure.