Fix unnecessary rig re-open cycles by refactoring rigctl_parse return values#614
Merged
mdblack98 merged 1 commit intoMar 17, 2021
Merged
Conversation
Collaborator
Author
|
That was fast! Thanks :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The current command loop in both
rigctland therigctldserver perform rig close/open cycles in case of hardware or I/O errors. However, the logic to detect these errors is not very accurate, asrigctl_parsereturns value2(indicating re-open is required) for allRIG_E*errors fromrig_commands. There are many errors, e.g. "invalid parameter", which do not indicate an issue in the rig communication and re-opening the rig is totally unnecessary in this case.Re-opening the rig causes real trouble when there are multiple clients connected to
rigctldserver and one of the clients issues a command resulting in "invalid parameter" error, for example. This will lead to the rig being unavailable for some time all other clients. See #612 for a more detailed explanation.This PR includes:
New, more deterministic return values in
rigctl_prasefunction. The function returns the negativeRIG_E*errors as they are, usesRIG_OKfor indicating a successful command and two positive integersRIGCTL_PARSE_END(1) andRIGCTL_PARSE_ERROR(2) to indicate end-of-stream and other internal parsing errors -- both of which will make the command loop exit.New macro
RIG_IS_SOFT_ERRCODEfor determining if a particular error code is a "soft error", indicating it is not a hardware or I/O fault and cannot be potentially fixed by re-opening the rig. The soft errcodes are: RIG_EINVAL, RIG_ENIMPL, RIG_ERJCTED, RIG_ETRUNC, RIG_ENAVAIL, RIG_ENTARGET, RIG_EVFO and RIG_EDOM.Some fixes to Icom backend to remove ACK/NAK detection in commands that get/read data from the rig, as the ACK/NAK bytes are present in responses to commands that set/change the rig state only.