Add generic calculation for translating from sample space to lab space and use for i23 OAV

[JamesDoingStuff]

Fixes #1168 and fixes #1232

Instructions to reviewer on how to test:

1. Confirm new tests pass
2. Confirm dodal connect i23 connects the goniometer
3. Confirm docs are good on the utility function for someone not that familiar with the problem

Checks for reviewer

• Would the PR title make sense to a scientist on a set of release notes
• If a new device has been added does it follow the standards
• If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly
• Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

[codecov]

Codecov Report

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

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1215   +/-   ##
=======================================
  Coverage   98.17%   98.17%           
=======================================
  Files         222      222           
  Lines        8158     8160    +2     
=======================================
+ Hits         8009     8011    +2     
  Misses        149      149           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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

[rtuck99]

"when the sample is at 90 deg" this is a confusing sentence and seemingly contradictory to the argument name.

[rtuck99]

So I think this doc comment can be made clearer. For me, I find the naming of the args a bit confusing, because the frame of reference of the angle in the name isn't clear and is contradicted by the arg description (although I think I understand your intent with the sign flip, it is I think unnecessarily confusing).

I think it's better to describe the geometric assumptions in the doc comment, and label the axes neutrally, because this function is applicable to a variety of different geometries and labelling the axes things like whether any given axis is right- or left- handed, up or down etc are essentially not relevant, as long as the inputs conform to the documented requirements it should give the right answer.

While I was reviewing this, to make it easier for myself to follow the implementation, I renamed the args motor_i, motor_j and motor_theta. I tried to come up with some wording for the comment that would summarise less ambiguously what the function does, what I came up with was this:

Given a signal that controls a motor in a rotation axis and two other
signals controlling motors on a pair of orthogonal axes, these axes being in the
rotating frame of reference created by the first axis,
create a derived signal that is a projection of the two axes in the non-rotating
frame of reference.

The projection is onto the axis defined by i when the rotation angle is 0.

For me I think this is clearer, although it is quite technical and I think you should retain the illustrative text in the comment because not everyone prefers this kind of explanation and it is generally helpful. Also I think my sentence structure is still a bit clunky and overly long. Perhaps breaking it up into bullet points would help, I'm open to improvements in that regard. For me, the important facts are that:

• There are two frames of reference, one is stationary, one is rotating
• One input is a motor controls the angle between the two frames of reference
• The other inputs are on orthogonal axes in the rotating frame, with the given orientations at specified angles of theta
• The output is a motor which is a projection onto the stationary frame parallel to i when theta == 0

Also, I think it might be better if the rotation motor was moved to the first argument of the function, because it's the important thing that controls how the other two axes are interpreted.

@olliesilvester do you think my text above is helpful?

[olliesilvester]

I think your wording is a bit more clear. I would nitpick it slightly to

    Given a signal that controls a motor in a rotation axis and two other
    signals controlling motors on a pair of orthogonal axes, these axes being in the
    rotating frame of reference created by the first axis,
    create a derived signal that is a projection of the two orthogonal axes and is in the lab
    frame of reference.

    The projection is onto the axis defined by i when the rotation angle is 0.

This function is going to be hard to understand however good the wording is. Ideally we would have a diagram to explain it

[DominicOram]

Completely agree, sounds a lot better. Thanks!

[rtuck99]

Can we please use standard google pydoc notation here: https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings? It helps the generated API docs format nicely.

You can also use doctest style examples: https://docs.python.org/3/library/doctest.html
The advantage is that the examples then get checked for correctness which helps make sure that the docstrings stay up to date. However although the formatting is nice if you don't want the tests to actually run, then you can use # doctest: +SKIP

[rtuck99]

These fixtures are the same?

[rtuck99]

Implementation looks fine, some comments about documentation which I think does need to tightened up a bit mainly because coordinate systems are always confusing.
Also need to sort out the issue with the unit test fixture. Some more test cases for the round trip test would also be good.

[rtuck99]

Approved