Slew to Target, Center Target, and Rotate Target Interaction

Issue #896 closed
Peter Morse created an issue

Create a sequence in the SimpleSequencer. Set the Rotate Target option to true. Save the sequence. Load the sequence. In addition to Rotate Target, Slew to Target and Center Target will now be set.

In the process of saving the target the code does a transformation to CaptureSequenceList. If you look at these three parameters in this class there is some code in each of them that can conditionally change the other two. Firstly, it’s questionable coding style to have parameters have side effects to other parameters. Second, why are these side effects there? Each parameter is independent in the physical world so I don’t understand the interdependence in the code.

If there is some valid reason for this interaction then the UI for these three conditions needs to be changed to reflect the interaction immediately rather than have them change by magic when you save/load a sequence. Either way it’s an easy fix. I assume this interdependence is wrong but someone with some history with this code will need to make the call.

Comments (11)

  1. Stefan B repo owner

    Those options build on one another.

    You can’t center a target without slewing to it - and it doesn’t make much sense to rotate a target without first slewing and centering it. That’s why these are connected with each other. Layout can of course be improved here, the data structure is mostly in place due to backwards compatibility for 1.10. otherwise it would be different.

  2. Peter Morse reporter

    It is not required to do an automated slew or center before a rotation. There are at least two cases where a user might do only a rotation. 1. The user has a high end mount with a good sky model and manually slews to the target. Then wants NINA to do the rotation. 2. The user wants to image the same target at different rotations. The second target would only require a rotation.

    At least two similar cases exist for doing a center without a slew. You can center a target without slewing to it first - the plate solver just does a large first slew correction.

    So, slew, center, and rotate are independent physical operations that the user should be able to select without side effects to the others. Which is not to say that a user couldn’t mess up and select only rotate when they should select all three. If NINA needs to save the user, then error messages should be used rather than these unseen side effects.

    Therefore having the data structures as three binaries is fine. It’s the artificial side effects that are the problem.

  3. Dale Ghent

    The plate solve must be done to get the sky angle for rotation, so the cost of the center is practically nothing as its information comes along for the ride. It’ll essentially be a no-op if the solved coordinates are within the configured Pointing Tolerance. I would also argue that those who are using high-end mounts that have pointing/tracking models at their disposal are more likely to be using Advanced Sequencer anyhow and not care about Simple. Again, this is looking for problems where they don’t exist.

  4. Peter Morse reporter

    Please reread the original problem statement. You create a sequence, save and load it and you end up with something else than you started with. If you run the sequence before the save, it will behave differently than after the load. The rest of the original post was stating the cause of the problem. Apparently you think that having the UI mysteriously changing a sequence is not an actual problem? Hardly a case of “looking for problems where they don’t exist.”

    Getting the sky angle from the plate solve is fine but why do a center when the user did not request it? The interface is pretty clear about the three operations - why muddle it up in the code? NINA should not be doing unrequested operations considering that the interface has the operation turned off.

  5. Stefan B repo owner

    That those toggles are not interlinked with each other currently is an oversight. They should be, as described in my first comment.

  6. Dale Ghent

    Again, it’s not an actual problem. The center is a simple comparison for tolerance. It costs nothing when you’re already doing a solve for rotation. And I challenge you to come up with any plausibly acceptable reason that a person would specify coordinates to point at and not care about pointing accuracy. The pedantic view is unwarranted and actually changes the existing and long-standing behavior. In that view, you’re actually creating a problem instead of “fixing” one.

  7. Peter Morse reporter

    I already gave at least two real cases. So apparently you think the UI problem is not an actual problem? That is it OK to have a save/load change the sequence? I’m not sure what problem I creating. The sequence runs differently before and after the load - I think that is a problem.

    I don’t think this problem existed before the Advanced Sequencer was included because the old code just did a direct serialize to/from xlm and not the translation through CaptureSequenceList. I think there is a backward compatibility problem. So I’m not creating a problem, I’m pointing out a real issue. Changing the UI to match CaptureSequenceList does not address the backward compatibility. Keeping the one to one correspondence between the functions and the controls makes the most sense. What did the Advance Sequencer add that made this change necessary?

  8. Stefan B repo owner

    As I said the UI part is an oversight, that i have fixed now by interlinking those toggles together, like it was in 1.10, to have a similar user experience - and also only take the most relevant instruction for the advanced sequencer.

    If we want to display this part differently alltogether is another topic of discussion.

  9. Stefan B repo owner

    Oh and a solved rotation without a prior center is currently not available in the advanced sequencer, which is why having just rotate without slew & center not being an option here.

  10. Log in to comment