TprTrigger - Default timing_mode

[slactjohnson]

Current Behavior

The TprTrigger class forces you to specify whether you are using it for LCLS1 or LCLS2 timing through an enum, rather than a string or integer argument. There is no default.

Expected Behavior

Given that the TPR was designed for LCLS-2, the LCLS-2 mode should be the default.
This enum is a little annoying and verbose. I feel that a string parameter would be easier to use.

Context / environment

I am trying to dynamically create device classes from a yaml config file. I can't think of a good, re-usable way to pass this enum to the class when I'm pulling all of my data from a text file.

Suggested Solution

Add a default state, and ideally change the enum to a simple string.

[tangkong]

A string parameter is easier pass around but the enum is meant to restrict us fully to a known subset of options. I think I agree that LCLS2 could be the default, but I think it's worth keeping the enum (or if we do pass in a string, check that the enum has that value)

Your context seems like a job for happi. Is there something stopping you from loading these devices from happi? Or making your own happi db.json and loading from that?

[slactjohnson]

My plan was to remove the enum but leave in the code that checks that we've chosen LCLS1 or LCLS2, which is effectively all that enum is doing. My use-case would be resolved with simply adding a default case, so I'll leave the enum in and do that.

I had not considered happi for this but that might work. The context for what I'm working on is this issue: pcdshub/opcpa-tpr-config#21

This application was originally intended to control only Tpr channels, but now the requirements are getting more complicated and necessitate interacting with other devices/PVs.

I was thinking about a config file structure like this, where I could instantiate effectively any class and then use the configuration data in the file to set it up as required:

devices:
  ch0:
    desc: "what this is"
    base: "my:base:pv"
    class: "pcdsdevices.tpr.TprTrigger"
    kwargs:
      channel: 0
      timing_mode: LCLS2
      name: "name"
  ch1: 
    ....

But thinking more, this kind is effectively what happi does. D'oh.
These are not traditional "beamline" devices, so I hadn't considered using happi. But I kind of like having a separate db.json file. I'll try this out.

[tangkong]

I'm happi to help you get that set up, if you run into a wall at any point

[slactjohnson]

Nice. :)

I'm happi to help you get that set up