Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
keithralphs opened this issue Jun 24, 2024 · 8 comments
Labels
enhancement New feature or request rest api Potential REST API changes

Comments

@keithralphs
Copy link
Contributor

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:

  • minimum and/or maximum values for numerical quantities where appropriate
  • default values for all types where appropriate
  • resolution valuies for numerical quantities where appropriate - e,g. when changing floating point values, the minimum step a UI should support
  • multiple selection indication for lists of options e.g. to indicate its is valid to select more than one detector

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.

@keithralphs keithralphs added the enhancement New feature or request label Jun 24, 2024
@callumforrester
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Collaborator

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.

@keithralphs keithralphs added the rest api Potential REST API changes label Jun 25, 2024
@DiamondJoseph
Copy link
Collaborator

DiamondJoseph commented Jun 25, 2024

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
Copy link
Collaborator

@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
Copy link
Collaborator

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

@callumforrester
Copy link
Collaborator

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
Copy link
Collaborator

DiamondJoseph commented Jun 27, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request rest api Potential REST API changes
Projects
None yet
Development

No branches or pull requests

3 participants