nexdome / Firmware

NexDome Dome and Shutter Motor Kit Firmware
https://www.nexdome.com
Other
3 stars 6 forks source link

Changing slew destination without waiting for completion #28

Closed ghilios closed 3 years ago

ghilios commented 4 years ago

I'm integrating NexDome into NINA, which is open source AP imaging software. I'd like to be able to change the destination of an active slew request, but it doesn't seem like there is a way to do it either in the ASCOM driver or the firmware. My observation so far is that the firmware ignores @GAR commands if a slew is in progress, and the only way to stop it is to abort a slew first (@SWS). This has a few downsides: 1) If a shutter operation is in progress, that is aborted to and the shutter is left in an error state (you have to fully close before you can try opening again) 2) The movement is abrupt 3) Perhaps the instant stop without deceleration is bad for the hardware if done regularly?

Would it be possible to add support for some combination of the following? 1) Soft abort with deceleration 2) Accepting new slew requests to change the destination without requiring a stop first 3) Separating shutter stop from dome stop, so they can be controlled separately if needed

NameOfTheDragon commented 4 years ago

Closing issue as the resolution was identified as external (ASCOM configuration)

ghilios commented 4 years ago

Tim, can you help me understand the resolution? What does "ASCOM configuration" mean in this case? Does the firmware actually support this, and would a direct driver (not the provided ASCOM driver) be able to do this?

NameOfTheDragon commented 4 years ago

My comment might have been a bit unclear, I actually thought you were asking for something slightly different. OK, let's try again.

The firmware as designed expects to complete the operation in progress before allowing a new one. Therefore it cannot change the destination of a 'slew in progress'. You can argue that it should behave differently but that's how its designed. You either have to get the correct destination to start with, or cancel the slew and start a new one. That's unlikely to change because changing the slew destination can upset things like acceleration and stopping distances.

The firmware is at the limit of available code space so it might not be possible to charge things now, however if you talk to Babak and he's willing to fund the work, then I'd be prepared to re-examine this to see what can be done.

NameOfTheDragon commented 4 years ago

Re-opened pending further discussion.

ghilios commented 4 years ago

Thanks for clarifying. Perhaps I can take a look sometime as well, if it matters enough to NINA usability.

NameOfTheDragon commented 4 years ago

@ghilios I've just been working on the firmware and I did some tests on this.

It does seem to be possible to set a new destination while the dome is in motion and it works up to a point, even decelerating and reversing if necessary, but the issue seems to be with stopping. It does eventually stop at the right position, but you usually end up going around twice, which is probably not what you want.

I think this is because of how the step generator keeps track of step position. At the lowest level steps are mapped onto a continuous number line, so you are at A and you want to go to B, you need to move X = B - A steps. At the next level up that's a problem because the dome is circular and its position wraps from 360 degrees (or about 55000 steps) back to 0. The step generator has no way of "wrapping" its step position, so to get from say step position 55000 to step position 10, the step generator is commanded to move to position 55010. The step position is constrained by the limits of an integer in memory and needs to be kept within this range, so after motion stops, the step position is "normalized" back to a small positive integer. Because this only happens after motion has stopped, the calculations seem to go a bit wrong if the target step position is changed on the fly.

The step generator could have been done differently but its supposed to be general purpose and one of the primary design goals was to keep step generation as simple as possible, so it can be done in an interrupt service routine. This is how I get my high step rate with consistent square wave output. So step generation is limited to incrementing or decrementing the position and there's no provision for 'wrapping'.

It might be possible to change this behavior, but code space comes into play. Currently, there is about 4% free on the rotator so it might be do-able. If you would like to have a go and submit a pull request then I'm totally open to that, but for me to work on it I need to receive instructions from NexDome, so it is they who you have to persuade. Perhaps @nexdome would care to comment?

If you are using the ASCOM driver then changes will be needed there too, because the state machine currently only allows a slew command when the dome is stationary. I think that would be a reasonable straightforward change though. The goal here is to arbitrate between conflicting commands from multiple clients. I chose the "first command wins" approach, I never anticipated a single client conflicting with itself ;-)

ghilios commented 4 years ago

I appreciate all of this context, and I do understand that you probably can't spend too much more time on it without Babak asking you to. I'll experiment with this a bit and see if I can make some headway.

NameOfTheDragon commented 3 years ago

@ghilios I've just tried this out again using the 4.0.0 beta firmware and it seems to be working now. It looks like the reason it didn't work before might have been related to the "never ending slew" bug. I haven't tested it extensively, but the firmware does seem to handle slew pre-emption using both @GAR and @GSR. You might want to experiment with it.

ghilios commented 3 years ago

Woah this sounds like a game changer! I've been bogged down at work, and I expect to have some time to test this in the next few days. Thanks!

NameOfTheDragon commented 3 years ago

Closing for now so I can release 4.0.0 with a clean slate. I am going to assume this is working unless you tell me otherwise. You are welcome to re-open if you need to.