Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

Create panda subdirectory within panda plan #1371

Merged
merged 7 commits into from
May 16, 2024
Merged

Conversation

olliesilvester
Copy link
Contributor

@olliesilvester olliesilvester commented May 9, 2024

Fixes #1339

Link to dodal PR (if required): #516
(remember to update setup.cfg with the dodal commit tag if you need it for tests to pass!)

To test:

  1. Confirm changes are sensible and tests pass

@olliesilvester olliesilvester marked this pull request as draft May 10, 2024 12:29
@olliesilvester olliesilvester changed the title Small changes to make sure panda subdirectory exists (WIP )Small changes to make sure panda subdirectory exists May 10, 2024
@olliesilvester olliesilvester changed the title (WIP )Small changes to make sure panda subdirectory exists Create panda subdirectory within panda plan May 13, 2024
@olliesilvester olliesilvester marked this pull request as ready for review May 13, 2024 13:36
d-perl
d-perl previously approved these changes May 16, 2024
Copy link
Contributor

@d-perl d-perl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, thank you!

Copy link
Collaborator

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of comments in code. For interest, what are the names of the files that panda will write out?

)
panda_directory = Path(parameters.detector_params.directory)

# Don't create directory once implemented in ophyd-async
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should: Does this have an ophyd_async issue we can link to?

get_directory_provider().update(
directory=Path(parameters.detector_params.directory)
)
panda_directory = Path(parameters.detector_params.directory)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should: I think parameters.storage_directory is where we get the detector directory from originally so we should probably use that here rather than digging into the detector.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should: I'm also mildly concerned that people will get detector and panda data confused. Can we put it in a panda subfolder for now?

@d-perl d-perl dismissed their stale review May 16, 2024 12:51

missed some things

@DominicOram DominicOram dismissed their stale review May 16, 2024 15:00

I've done the changes I wanted

@DominicOram DominicOram requested a review from d-perl May 16, 2024 15:00
Copy link
Contributor

@d-perl d-perl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@DominicOram DominicOram merged commit 031dd67 into main May 16, 2024
10 checks passed
@DominicOram DominicOram deleted the 1339_fix_panda_pcap branch May 16, 2024 15:39
olliesilvester pushed a commit to olliesilvester/mx-bluesky that referenced this pull request Aug 23, 2024
…Source/1339_fix_panda_pcap

Create panda subdirectory within panda plan
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panda: Write positions for every gridscan
3 participants