DiamondLightSource / blueapi

Apache License 2.0
6 stars 6 forks source link

Add limits, default values, resolution and control hint fields to the enhanced JSONSchema parameters for plans #519

Open keithralphs opened 4 months ago

keithralphs commented 4 months ago

For the enumerated version of the /plans and /plans{name} endpoints (see #518 ), extra contextual fields should be added to the properties describing each plan parameter to support validation when used by UI generation systems liike JSONForms. These should include at least:

This will support validation of submitted parameters by User interfaces improving the feedback in the user experience and preventing the submission of invalid requests to execute the plan. It could be argued that these should be added to existing version of the endpoints too, as this would alloe the CLI to support such validation too, but I am leaving that as a decision to be discussed.

callumforrester commented 4 months ago

The first 3 bullet points will have to be user-specified in the plan signature somehow, the fourth is a bit separate and possibly warrants slitting out into its own issue.

keithralphs commented 4 months ago

The first 3 bullet points will have to be user-specified in the plan signature somehow, the fourth is a bit separate and possibly warrants slitting out into its own issue.

I think it's usually just a case of adding "uniqueItems": true to any arrays so may also be able to be user specified, or be based on the type of the param?

callumforrester commented 4 months ago

multiple selection indication for lists of options e.g. to indicate its is valid to select more than one detector

The canonical way to do this is to put set[Detector] rather than list[Detector] in the plan signature, this will automatically add a uniqueItems: true to the schema. No further action needed on this except to modify plan signatures as-needed.

DiamondJoseph commented 4 months ago

Bullet number 1 should be on the Device in question IMO, because it is a property of the signal

  1. Get plan names
  2. Get plan for name
  3. Get devices that are of <device type> for parameter
  4. Device tells you limits
  5. .... magic, witchcraft probably
  6. UI autopopulates the correct limits

So plan field would have to become something approximating set_point = field(*somehow references the signal of <device type>*) for that to get autopopulated.

callumforrester commented 4 months ago

@DiamondJoseph I think that's some very complicated validation, and maybe we just provide a hook for it and say "if you want that level of validation, you need to write your own code"

DiamondJoseph commented 4 months ago

I agree, but limits are not really a property of a plan.

callumforrester commented 4 months ago

Why not? Maybe not motor limits, but there are other use cases of needing to define arbitrary limits on plan parameters (e.g. this must be positive etc.)

The question is how we tie some specific parameters to motors

DiamondJoseph commented 4 months ago

I think "number must be positive", "list must have at least 1 element" are all validation and sensible to be properties of plans (and simply to accomplish with fields?). But when the plan is trying to e.g. set an exposure time (which must be validated at >0), the limits on the exposure time depend on a/multiple devices that isn't a property of the plan.