Add LITERAL rotator range type and consolidate FULL range movement#299
Add LITERAL rotator range type and consolidate FULL range movement#299daleghent wants to merge 7 commits into
Conversation
| } | ||
|
|
||
| // Focuser position should be in [0, 360) | ||
| // Focuser position should be in [0 .. 360] |
|
|
||
| public float GetTargetMechanicalPosition(float position) { | ||
| // Focuser position should be in [0, 360) | ||
| // Focuser position should be in [0 .. 360] |
| throw new NotImplementedException(); | ||
| break; | ||
|
|
||
| case Core.Enum.RotatorRangeTypeEnum.FULL: |
There was a problem hiding this comment.
This optimization looks good for sky-position-angle moves, but I don't think it belongs in GetTargetMechanicalPosition(). Callers like MoveRotatorMechanical and the equipment UI are asking for an explicit hardware angle, so changing 100 to 280 because it is a closer reciprocal can violate user intent..
I'd keep mechanical FULL literal and move the reciprocal optimization into GetTargetPosition() for sky-angle commands only, with LITERAL disabling that optimization if needed, what do you think?
There was a problem hiding this comment.
Agree. I've moved FULL processing to GetTargetPosition() and had copilot create documentation for both GetTargetPosition() and GetTargetMechanicalPosition()
|
The CI failures look like stale test expectations from the refactor: FULL reciprocal adjustment moved into GetTargetPosition(), but these tests still mock GetTargetPosition() returning the unadjusted target, so the sequence code computes 260 - 160 = 100 while the assertions still expect the old inline-adjusted -80. |
| /// <exception cref="Exception"> | ||
| /// Thrown when the rotator has not been synced. Sync must be performed before calling this method. | ||
| /// </exception> | ||
| public float GetTargetPosition(float position) { |
There was a problem hiding this comment.
This can choose the closer 180°-equivalent target, but the method still returns it as an absolute sky angle. The callers do not all turn that back into the shortest signed physical movement.
Example with zero sync offset: current position is 0° and requested sky angle is 100°. This branch selects the equivalent target 280°, which is only closer if commanded as -80°. But the direct move path passes the returned absolute target onward, and the sequencer path computes targetRotation - orientation, so this can still become +280° instead of -80°.
2 participants
🚀 Purpose
This PR introduces a new LITERAL rotator range type, which commands the rotator to travel to the given position without regard to proximity to the reciprocal angle. It also consolidates FULL range type into
RotatorVM, removing the diplicative closest-reciprocal test and modification from theCenterAndRotateandSolveAndRotateclasses. This makes all rotator actions consistent for FULL type movements regardless of caller.🧪 How Was It Tested?
ASCOM OmniSim rotator
🤖 AI / Tooling Disclosure
Test case generation by Claude Sonnet 4.6
✅ PR Checklist
RELEASE_NOTES.mdupdated (if applicable)