Modified goToTarget2 such that it only slews if coordinates valid

[noralinn]

Please, verify the verification and close if you think it is safe.

Question: Should MinAlt be hardcoded in goToTarget2 or would it be better to also get it from the mount object or elsewhere?

                % verification:
                if Alt<MinAlt
                    fprintf('Error: Target requested altitude (%f) is below limit (%f)',Alt,MinAlt)
                    MountObj.LogFile.write(sprintf('Error: Target requested altitude (%f) is below limit (%f)',Alt,MinAlt));
                    Flag = false;
                elseif abs(Aux.HA_App)>MountObj.HALimit
                    fprintf('Error: Requested HA (%f) is out of allowed range',Aux.HA_App)
                    MountObj.LogFile.write(sprintf('Error: Requested HA (%f) is out of allowd range',Aux.HA_App));
                    Flag = false;
                elseif ~isfinite(OutRA*OutDec)
                    fprintf('Error: RA (%f) or Dec (%f) not finite',OutRA, OutDec)
                    MountObj.LogFile.write(sprintf('Error: RA (%f) or Dec (%f) not finite',OutRA, OutDec));
                    Flag = false;
                else
                    % move mount
                    Flag = true;
                    MountObj.goTo(OutRA, OutDec, 'eq');
                end

[EastEriq]

There is already a MountObj.MinAlt, and a specific check for it in XerxesBinary.goTo (as well in all variations of the physical mount classes) (see e.g. https://github.com/EastEriq/LAST_XerxesMount/blob/a899ba4e1be89bd4973336eb5c9e458a53d5b927/%2Binst/%40XerxesMountBinary/goTo.m#L38). I don't see a reason for adding a separate value in higher level code. It is also logical that these safeties are handled by the driver, in physical driver coordinates.

As for HA limits, they are even flashed into the controller, further duplication is also unnecessary and error prone. The test as written above is anyway wrong, because there is no such property HaLimit, but MotorHALimits returning both extremal values.

As for isfinite - really, who is supposed to pass to goTo infinite values? They would result out of limits anyway. The only caveat would be about passing a NaN, but then use isnan for clarity. (which leads me to think, what happens as of now if I pass NaNs? Let me check and I'll tell you)

[noralinn]

I'm in favour of removing any unnecessary checks and letting a lower-level function deal with them.

I guess the behaviour we want for invalid coordinates is that the function returns Flag=0, doesn't slew and maybe prints a message and writes to the log. @EranOfek do you agree?

I added the ~isfinite statement, because OutRA and OutDec were NaNs due to a bug in j2000toapparent. I think that changing to isnan is fine. But maybe it would be even better to check whether RA is a float between 0 and 360 and Dec between -90 and 90 Deg? Or is that already covered by goTo?

[EastEriq]

This is the current behavior:

>> Unit.Mount.goTo(NaN,-15)
{inst.XerxesMountBinary} position cannot be reached given the axes limits! Not moving.
{inst.XerxesMountBinary} position cannot be reached given the axes limits! Not moving.

>> Unit.Mount.goTo(40,NaN)
{inst.XerxesMountBinary} position cannot be reached given the axes limits! Not moving.

>> Unit.Mount.goTo(NaN,NaN)
{inst.XerxesMountBinary} position cannot be reached given the axes limits! Not moving.
{inst.XerxesMountBinary} position cannot be reached given the axes limits! Not moving.

We could be overzealous in printing more descriptive messages, but there should be no risk.

[EastEriq]

@noralinn , btw: you're making these changes in mastrolindo, but we are working on all this in branch j2000inHeader: https://github.com/PolishookDavid/LAST_OCS/blob/j2000inHeader/%2Bobs/%40mount/goToTarget2.m