Skip to content

Conversation

@oliwenmandiamond
Copy link
Contributor

@oliwenmandiamond oliwenmandiamond commented Jan 28, 2026

Fixes #ISSUE

Instructions to reviewer on how to test:

  1. Check dodal connect works
  2. Check tests pass

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

codecov bot commented Jan 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.15%. Comparing base (07c1416) to head (e13e2a2).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1873   +/-   ##
=======================================
  Coverage   99.14%   99.15%           
=======================================
  Files         304      308    +4     
  Lines       11509    11578   +69     
=======================================
+ Hits        11411    11480   +69     
  Misses         98       98           

☔ 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.

@oliwenmandiamond oliwenmandiamond marked this pull request as ready for review January 28, 2026 12:08
@oliwenmandiamond oliwenmandiamond requested a review from a team as a code owner January 28, 2026 12:08
Base automatically changed from add_sample_stage_for_i09 to main January 28, 2026 13:30
@oliwenmandiamond
Copy link
Contributor Author

@Villtord I have added the configuration back. I have also added perp and long signals which are ported from the GDA equivalent

i05 - combined_gonio.xml

	<bean id="salong" class="uk.ac.gda.core.virtualaxis.CombinedManipulator">
		<property name="scannables">
			<list>
				<ref bean="sax" />
				<ref bean="say" />
			</list>
		</property>
		<property name="calculator">
			<bean class="uk.ac.gda.arpes.calculator.SaLongCalculator" />
		</property>
	</bean>

	<bean id="saperp" class="uk.ac.gda.core.virtualaxis.CombinedManipulator">
		<property name="scannables">
			<list>
				<ref bean="sax" />
				<ref bean="say" />
			</list>
		</property>
		<property name="calculator">
			<bean class="uk.ac.gda.arpes.calculator.SaPerpCalculator" />
		</property>
	</bean>  

i05 - gonio.xml

	<bean id="sa" class="gda.device.scannable.scannablegroup.ScannableGroup">
		<property name="groupMembers">
			<list>
				<ref bean="sax" />
				<ref bean="say" />
				<ref bean="saz" />
				<ref bean="sapolar" />
				<ref bean="satilt" />
				<ref bean="saazimuth" />
				<ref bean="salong" />
				<ref bean="saperp" />
			</list>
		</property>
	</bean>

Can you please review to make sure I have done this part correctly? From what I can tell, the angle is always static at 50 degrees? Should they maybe be added as helper functions like create_axis_perp_to_rotation so anyone can add them onto a device?

Copy link
Contributor

@Villtord Villtord left a comment

Choose a reason for hiding this comment

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

So, turned out virtual axis on Nano are not used anymore, I will leave my comments anyway



class I05Goniometer(XYZPolarAzimuthTiltStage):
"""Six-axis stage with a standard xyz stage and three axis of rotation: polar,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""Six-axis stage with a standard xyz stage and three axis of rotation: polar,
"""Six-physical-axis stage with a standard xyz translational stage and three axis of rotation: polar,

- `long`: Translation along the longitudinal direction of the rotated in-plane
coordinate frame defined by ``rotation_angle_deg``.

- `perp`: Translation perpendicular to `long` within the x-y plane.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `perp`: Translation perpendicular to `long` within the x-y plane.
- `perp`: Translation along the rotated Y-axis.

Comment on lines +22 to +23
- `long`: Translation along the longitudinal direction of the rotated in-plane
coordinate frame defined by ``rotation_angle_deg``.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `long`: Translation along the longitudinal direction of the rotated in-plane
coordinate frame defined by ``rotation_angle_deg``.
- `long`: Translation along the rotated X-axis.

Comment on lines +27 to +31
The `perp` and `long` axes are derived from the underlying x and y motors using a
fixed rotation angle (default 50 degrees). From the user's point of view, these
behave as ordinary orthogonal Cartesian translation axes aligned with physically
meaningful directions on the sample, while internally coordinating motion of the x
and y motors.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The `perp` and `long` axes are derived from the underlying x and y motors using a
fixed rotation angle (default 50 degrees). From the user's point of view, these
behave as ordinary orthogonal Cartesian translation axes aligned with physically
meaningful directions on the sample, while internally coordinating motion of the x
and y motors.
The `perp` and `long` axes are virtual axes derived from the underlying x and y motors using a
fixed rotation angle (default 50 degrees). Rotation angle corresponds to an angle between analyser axis and X-ray beam axis. From the user's point of view, these virtual axes
behave as ordinary orthogonal Cartesian translation axes aligned with the incoming X-ray beam (long) and perpendicular to it (perp), while internally coordinating motion of the x (perpendicular to analyser axis)
and y (along analyser axis) motors.

Comment on lines +33 to +34
Unlike sample-frame axes that rotate with a live rotation motor, these axes are
defined at a constant orientation set by `rotation_angle_deg`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Unlike sample-frame axes that rotate with a live rotation motor, these axes are
defined at a constant orientation set by `rotation_angle_deg`.

angle_deg: float | SignalR[float],
clockwise_frame: bool = True,
) -> tuple[SignalRW[float], SignalRW[float]]:
"""Create virtual i/j signals representing a Cartesian coordinate frame
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""Create virtual i/j signals representing a Cartesian coordinate frame
"""Create virtual i/j axes representing a Cartesian coordinate frame

Comment on lines +10 to +11
"""Six-axis stage with a standard xyz stage and three axis of rotation: polar,
azimuth, and defocus.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""Six-axis stage with a standard xyz stage and three axis of rotation: polar,
azimuth, and defocus.
"""Six-physical-axis stage with a standard xyz stage, 2 axis of rotation: polar,
azimuth and one extra tranlastional axis defocus.

Comment on lines +17 to +20
Horizontal and vertical translation axes in the sample frame.
These axes are derived from the lab-frame x and y motors and rotate
with the azimuth angle, so that motion along `hor` and `vert`
remains aligned with the sample regardless of its azimuthal rotation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Horizontal and vertical translation axes in the sample frame.
These axes are derived from the lab-frame x and y motors and rotate
with the azimuth angle, so that motion along `hor` and `vert`
remains aligned with the sample regardless of its azimuthal rotation.
Horizontal and vertical virtual translation axes of the rotated sample frame.
These axes are derived from X and Y axes rotated
with the azimuth angle, so that motion along `hor` and `vert`
remains aligned with the gravity direction regardless of its azimuthal rotation.

Comment on lines +23 to +27
Longitudinal and perpendicular translation axes in the tilted sample
frame. These axes are derived from the lab-frame z motor and the
sample-frame `hor` axis, and rotate with the polar angle.
Motion along `long` follows the sample's longitudinal direction,
while `perp` moves perpendicular to it within the polar rotation plane.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Longitudinal and perpendicular translation axes in the tilted sample
frame. These axes are derived from the lab-frame z motor and the
sample-frame `hor` axis, and rotate with the polar angle.
Motion along `long` follows the sample's longitudinal direction,
while `perp` moves perpendicular to it within the polar rotation plane.
Longitudinal and perpendicular virtual translation axes in the rotated sample
frame. These axes are derived from the Z-axis and the
virtual `hor` axis, and depend on the polar angle.
Motion along `long` aligned with the analyser axis,
while `perp` moves perpendicular to it within the polar rotation plane.

Copy link
Contributor

@Villtord Villtord left a comment

Choose a reason for hiding this comment

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

Thanks, just a few comments.

Comment on lines +22 to +25
i_read: SignalR[float],
j_read: SignalR[float],
i_write: Movable[float],
j_write: Movable[float],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there a separation of read and write if there could be just one SignalRW ?

return float(rotation[0]), float(rotation[1])


def rotate_clockwise(
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you actually need 2 functions rotate_cw and rotate_ccw - you can rotate ccw using cw function by changing angle alpha either to (-1)alpha or to (2Pi-alpha)

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.

3 participants