Merging of changes from FRAM branch #66
Conversation
…devices) or CONN_CONNECTED (for clients?..) states, or it may block queSend and therefore device authorization in centrald
…mand (cmd) to send arbitrary message to the mount and receive the reply (displayed in the logs only). Usable e.g. for remotely aginging the mount, requesting its low-level status, etc.
…e actual values, and not the ones at readout start (as it may be after current script end + reset of parameters). Fixes potentially broken readout/transport of last frame of a script. GXCCD driver updated for that change. Probably, other drivers also should be reviewed and updated.
…hanges are applied, so we have the correct information on what exactly was started
…e to object being below horizon. Also, do not call info() from idle() (as it is called too often), but reduce idleInfoInterval instead.
… extended status line.
…om script. This fixes 'wait_target_move' command for the use cases when script controls the movement of telescope. The logic should probably be moved to target.ec::updateSlew() or so.
…anyway. And if unread, it may get stuck blocking everything else.
…anyway. And if unread, it may get stuck blocking everything else. Second try.
…e contains whitespaces
…and to 'invalid number of parameters' errors in the message log.
…seful for debugging. Also, displaying of next system state and time remaining till state change in rts2-mon status screen.
…let it properly honour print_millisec argument
…efore exposure start (when observationStarted() is still false).
…killed if it does not send anything to master process for a given time interval (10 minutes by default). Also, enabled logging of all communications with external scripts.
…rt what was actually received).
… create any number of variables from command line. Comments for variables may also be passed, by appending them to variable name with a comma. All sensors created this way will be weather devices.
…-writable for external sensor
| @@ -1494,7 +1494,11 @@ void Telescope::checkMoves () | |||
| } | |||
| else | |||
| { | |||
| maskState (TEL_MASK_CORRECTING | TEL_MASK_MOVING | BOP_EXPOSURE, TEL_NOT_CORRECTING | TEL_OBSERVING, "move finished without error"); | |||
| if ((woffsRaDec->wasChanged () || wcorrRaDec->wasChanged ())) | |||
There was a problem hiding this comment.
Is this really useful? This might solve a situation, when wait-offsets or wait-corrections were changed during the (just finished) movement. Anything defined before should already be included in this movement... After new movement started, no (astrometry) feedback is taken into account, so the only thing which can change is the wait-offsets (which is not very probable)...?
There was a problem hiding this comment.
Yes, I believe I made this exactly to solve such situations. Do not remember the details, maybe that was useful for solar system targets (which we observe a lot), or to applying small adjustments between exposures. It was definitely to prevent something real that caused trailed images in our setup (as the exposure was triggered on releasing the block, but the mount immediately set the block again and started to move)
There was a problem hiding this comment.
Hmmm, it's strange. This change is probably harmless, but still, IMO it's not systemic and might be confusing...
Also there were some errors in the teld, maybe this is not necessary to have it there now. Could you put there some log message and try to find it in your logfiles after several nights of observations?
There was a problem hiding this comment.
Probably the reason here is what tracking MPEC objects actually does not increase moveNum - so the corrections from previous pointings are still valid, and may arrive during the repointing. I added some logging to the code, let's see whether this path will be called...
lib/rts2tel/teld.cpp
Outdated
| @@ -2377,7 +2381,20 @@ int Telescope::startResyncMove (rts2core::Connection * conn, int correction) | |||
| } | |||
| } | |||
|
|
|||
| // all checks done, give proper info and set TEL_ state and BOP | |||
There was a problem hiding this comment.
I suggest keeping the original state of this (rejecting this particular change, ie following 14 lines).
I understand this was ment for speeding up the movement initiation, but, the original code is not time-consuming and sets all the complex teld states before the real movement is really initiated (which COULD be time-consuming).
Also, you didn't delete the original part of the code (startResync () etc below), which is probably just a consequence of incorrect merging.
There was a problem hiding this comment.
Yes, the original commit is here - karpov-sv@db0690b
It moves startResync() piece, and explains why it was done - not to speed things up, but to prevent accidental sending of EVENT_MOVE_OK before EVENT_MOVE_FAILED in case of movement error.
There was a problem hiding this comment.
Well, this definitely should not happen.
The EVENT_MOVE_* are part of executor code, there is a need to revise the states of the teld at all (to set the error states for more cases than it's now, to be consistent with the executor). I know about this issue - and I'll look at it soon.
I doubt that just changing the order (as it is in this commit) will solve all the issues.
For now, I suggest keeping this specific part of the code at the state of the main branch.
Or at least, postpone the merge a bit, I'll try to examine it ASAP :-).
src/db/tpm.cpp
Outdated
| @@ -250,67 +252,67 @@ int TPM::headline (rts2image::Image * image, std::ostream & _os) | |||
|
|
|||
| int TPM::printImage (rts2image::Image * image, WorldCoor * wcs, std::ostream & _os) | |||
| { | |||
There was a problem hiding this comment.
I don't really understand this change, tpm.cpp creates isolated binary...?
There was a problem hiding this comment.
It is very cryptic even to me. I don't remember touching this file at all, but it was definitely changed, in a commit that does not explain anything about it. So I suppose the whole file may be reverted to original state (but not the whole commit as it has other useful changes)
There was a problem hiding this comment.
Pushed a commit reverting this change
Pull request from actual branch where merging will take place.
Already has all implicit conflicts resolved.
Not even the compilation is tested yet :-)