Stage zocalo and centre outside of common gridscan

[olliesilvester]

Fixes #1299

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

Instructions to reviewer on how to test:

1. Do thing x
2. Confirm thing y happens

Checks for reviewer

• Would the PR title make sense to a user on a set of release notes

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.85%. Comparing base (d55fb90) to head (0ab3106).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1557      +/-   ##
==========================================
+ Coverage   92.83%   92.85%   +0.02%     
==========================================
  Files         152      153       +1     
  Lines        8606     8632      +26     
==========================================
+ Hits         7989     8015      +26     
  Misses        617      617              

Components	Coverage Δ
i24 SSX	78.60% <ø> (ø)
hyperion	98.22% <100.00%> (+<0.01%)	⬆️
other	98.32% <100.00%> (+<0.01%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[DominicOram]

Thanks, a couple of comments in code. Additionally:

As discussed, can we move all the logic for stage/unstage of zocalo and fetch_xrc_results_from_zocalo up into pin_tip_centre_then_xray_centre? This would mean that everything lower down than this doesn't need to care about centring.

Also as discussed, common_flyscan_xray_centre is now poorly named as it doesn't do an xray_centre it does a gridscan w/o centring. The docstring on it also mentions Optionally fetch results from zocalo after completing the grid scan, which isn't really related to the plan. This is a wider issue though because you then will also need to change create_parameters_for_flyscan_xray_centre. I have added to #366.

[olliesilvester]

@DominicOram Your review comments made this PR a lot better, thanks. It's also a lot bigger and harder to review unfortunately

[DominicOram]

Thanks, this is cleaner I think. Couple of comments

[DominicOram]

This really messes with the parameter type hierarchy. A SpecifiedGrid is of type GridCommon. So the code in create_parameters_for_flyscan_xray_centre basically takes one of these GridCommons, makes a new child object out of it by adding some extra bits then we add it back into this parent object? I'm not sure what a correct solution is without re-writing everything but can we at least add it into #1301 as a known code smell? And maybe a comment here pointing to that issue

[DominicOram]

DiamondLightSource/python-dlstbx#255 would also fix it

[olliesilvester]

I agree, it's nasty. We need a reference to an object that doesn't exist yet so that we can set-up the plan correctly at the entry point. Would a weak reference here be nicer? Eg
_specified_grid_ref: ReferenceType[SpecifiedGrid] | None = PrivateAttr(default=None)
It's still a weird but it feels cleaner to have just the one SpecifiedGrid object

[olliesilvester]

I'll add it into the linked issue regardless

[DominicOram]

Should: Were we previously changing aperture based on xtal size for i04? I didn't think this was something they wanted

[olliesilvester]

Thanks, you're right

[olliesilvester]

#1611

[DominicOram]

Thanks, I still think the way we're doing the parameters now isn't great but lets get this in.