Skip to content

Conversation

@DenSinH
Copy link
Contributor

@DenSinH DenSinH commented Nov 6, 2025

I hope this change does not seem too strange, we are actually kind of curious how other facilities are using the scan server, as we think we might be using it in a strange way (more like a "scripting server"). Basically, we want to be able to schedule certain tasks (which are usually started by clicking a button, which triggers remote procedures through PV writes, but potentially also other more complex tasks), and we figured the scan server does almost everything that we want to do, besides scheduling. We added a new "scan type" (ScheduledScan), which behaves like a queued ExecutableScan until its execution time has come, when it is "resubmitted" to the scan engine as a normal ExecutableScan, and executed (either queued or not depending on what the user configured). So long as the scans are still scheduled, they are not blocking. I also made it so the progress indicates the leftover waiting time, and the execution time is shown in the "Command" field.

I decided to put the ScheduledScans in the same queue as the ExecutableScans and the LoggedScans, so the "usual" operations still work on them. It required an extra ScanState, which I also added to scan.db in the examples folder as the comment seemed to suggest (?). I am not sure how to test if that still works. This change also adds some dependencies from the display runtime on the scan server (to add a "schedule task" context menu to ActionButtons with WritePV-type actions).

Again, we are very curious if this is something other facilities might like, and how the scan server is being used in general. Perhaps you have another suggestion to execute scheduled tasks with something that is already in phoebus, if you do not deem this a good fit for the scan server. The commits labeled [phoebus] are just fixes / improvements that may be cherrypicked into main anyway, and are unrelated to the scheduling, though related to the scan server.

Checklist

  • Testing:

    • The feature has automated tests
    • Tests were run
    • If not, explain how you tested your changes
  • Documentation:

    • The feature is documented
    • The documentation is up to date
    • Release notes:
      • Added an entry if the change is breaking or significant
      • Added an entry when adding a new feature

@kasemir
Copy link
Collaborator

kasemir commented Nov 6, 2025

As for the basic idea:

We added a new "scan type" (ScheduledScan), which behaves like a queued ExecutableScan until its execution time has come, when it is "resubmitted" to the scan engine as a normal ExecutableScan, and executed (either queued or not depending on what the user configured).

Seems like a good idea, with some questions:

If I submit a ScheduledScan to run at 01:00, this means at around 01:00 it'll be submitted as a plain queued scan, right? If there's nothing else pending, it will start at 01:00, but if there's already something else running that takes 5 hours, the scan scheduled for 01:00 will wait in the queue until ~06:00, correct?

If the scan server is restarted, any scheduled scans are lost just like any other queued scans are lost, correct?

As for the implementation:

public class ScheduledScan extends ExecutableScan {
    /** Time at which this scan should be queued / executed.  */
    private final LocalDateTime when;

Please use Instant, as is the case pretty much everywhere else you can prompt for input or display time in the local time zone, but internally we always deal with Instant

This change also adds some dependencies from the display runtime on the scan server (to add a "schedule task" context menu to ActionButtons with WritePV-type actions)

That's bad. So far, we had no dependency between the display and the scan, and it should stay that way.
We were always able to create scan related GUIs. Based on what the scan should accomplished, users need a GUI to enter 1 or 100 parameters. For example, to scan something over a start .. end range, you need a GUI with start, end, stepwise, maybe time to wait at each step etc. Those values are stored in (local) PVs. An action button then invokes a script to turn the info from PVs into a scan and submits that, typically using https://github.com/PythonScanClient/PyScanClient.
Similarly, please extend the PyScanClient to support submitting scheduled scans, and then call that from for example an action button script. No changes/dependencies needed to the display.

@DenSinH
Copy link
Contributor Author

DenSinH commented Nov 6, 2025

If I submit a ScheduledScan to run at 01:00, this means at around 01:00 it'll be submitted as a plain queued scan, right? If there's nothing else pending, it will start at 01:00, but if there's already something else running that takes 5 hours, the scan scheduled for 01:00 will wait in the queue until ~06:00, correct?

Correct, unless you scheduled it and mark it as unqueued, in which case at 01:00 it will run in the "parallel executor" (same as the normal unqueued scans), regardless of what else is running. The default is queued, as it is with normal scan executions.

If the scan server is restarted, any scheduled scans are lost just like any other queued scans are lost, correct?

Yes, we discussed this internally as well, that it is probably nice if the database behind the scan server stores all scans, not just logged scans.

As for the implementation:

public class ScheduledScan extends ExecutableScan {
    /** Time at which this scan should be queued / executed.  */
    private final LocalDateTime when;

Please use Instant, as is the case pretty much everywhere else you can prompt for input or display time in the local time zone, but internally we always deal with Instant

Will fix this, the reason I used LocalDateTime is because that is what was used for the deadline parameter as well. I do agree that it was terribly annoying to deal with the timezone stuff 🤪 and I was a bit confused why it was used at all.

This change also adds some dependencies from the display runtime on the scan server (to add a "schedule task" context menu to ActionButtons with WritePV-type actions)

That's bad. So far, we had no dependency between the display and the scan, and it should stay that way. We were always able to create scan related GUIs. Based on what the scan should accomplished, users need a GUI to enter 1 or 100 parameters. For example, to scan something over a start .. end range, you need a GUI with start, end, stepwise, maybe time to wait at each step etc. Those values are stored in (local) PVs. An action button then invokes a script to turn the info from PVs into a scan and submits that, typically using https://github.com/PythonScanClient/PyScanClient. Similarly, please extend the PyScanClient to support submitting scheduled scans, and then call that from for example an action button script. No changes/dependencies needed to the display.

Alright, I will have a look at this.

@kasemir
Copy link
Collaborator

kasemir commented Nov 6, 2025

OK on the queued/non-queued behavior.

Yes, the derby database could store commands as well as logged data, and that would allow restarts to keep the full scan info. If that's considered, it would be a separate project, so leave it as found for now.

I used LocalDateTime is because that is what was used for the deadline parameter

Sorry, that was a bad idea. I was too shocked by some maven issue to think straight. Should all be Instant, so please use that, and if you have any spare time, also fix the deadline.

@DenSinH
Copy link
Contributor Author

DenSinH commented Nov 6, 2025

A suggestion here was to use an SPI to be able to add context menu items to widget runtimes (this is what the added dependency was for), so that the scan client can register itself to action buttons for this scheduling. This saves us from having to use python scripts in every single action button for doing this kind of thing (which we preferably would not do).

@kasemir
Copy link
Collaborator

kasemir commented Nov 6, 2025

How do you submit a scan? Step 1 is that you need the scan commands. How do you assemble these? Typically, that requires a few parameters like start, end, ... values.
You thus need a script anyway to turn the values of PVs or data from a table into the list of scan commands.

@DenSinH
Copy link
Contributor Author

DenSinH commented Nov 6, 2025

Well, basically we'd like to just start a scan with a bunch of <set> commands for all the WritePV actions, nothing more complex than that. That would be the menu entry we would use in the action buttons. Any more complex behavior can't be added to an actionbutton (automatically) I think.

@kasemir
Copy link
Collaborator

kasemir commented Nov 6, 2025

If you want a button in the display to submit a (scheduled) scan to Set("pvname", 3.13), you can do that with a simple script.
Next, you'd like to have an action that doesn't write a constant 3.14 but gets that value from some local PV, or derives it from several other values.
Next you want to get the PV name from somewhere instead of hardcoding "pvname".
All that is possible with scripts.
Adding a dependency from the display to the scan modules just to support the first case doesn't make sense.

@DenSinH
Copy link
Contributor Author

DenSinH commented Nov 6, 2025

Agreed that that dependency should go away, that's why perhaps using an SPI (where the scan client can register a menu item) might be a solution, though I could also remove this feature and keep it only for us internally.

@kasemir
Copy link
Collaborator

kasemir commented Nov 6, 2025

Yes, you can add the SPI into your site-specific product.
But again what you want to hardcode into an action is basically this:

from scan import *
client = ScanClient('localhost')
cmds = [ Set('pvname', 3.14) ]
client.submit(cmds)

You can easily grow that script to get the value or PV name from somewhere else.
Or put it into your site library as

def OurScanBasedSet(pv_name, value):
  from scan import *
  client = ScanClient('localhost')
  cmds = [ Set(pv_name, value) ]
  client.submit(cmds)

and now your script becomes a one-liner.

@DenSinH
Copy link
Contributor Author

DenSinH commented Nov 7, 2025

I implemented the fixes / improvements you suggested and removed the dependencies. How we are going to add the task scheduling from the display runtime internally we shall see, perhaps with a python script, perhaps with some introspection magic, but it seems that it is not a great fit to upstream this in the end. I can squash some commits if you prefer?

By the way, LocalDateTime is still used in other places (for example internally in the DateTimePane), but I didn't want to break anything.

@kasemir
Copy link
Collaborator

kasemir commented Nov 7, 2025

Thanks for updating to Instant! Inside the DateTimePane it makes sense to use LocalDateTime because the user should interact with the local time, but what comes out is then DateTimePane::getInstant(). Don't want to compute a delay as "scheduled - now" using LocalDateTime because that might be unexpected during daylight savings time transitions.

Can you revert the changes to app/display/* (extra clock.png, ..)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants