Allow setting CW speed liberally in range 6..60 wpm #437
Conversation
This file contains 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
|
The idea sounds reasonable. As the old speed list was not configurable from logcfg.dat I think nobody used the chance to adapt it to its own preferences anyway.
|
The old coding had a fixed list of allowed CW speeds that were stepped through using PgDn/PgUp. While this might make sense for different step sizes (for example 2 wpm between 12 and 30, and 5 wpm above 30), it was not used as such. All steps were 2 wpm, and jumps from 12 to 6 and 50 to 60 at the very ends of the scale. For slow speed contest training, it does make sense to allow speeds between 6 and 12 wpm. Also, since Hamlib keying was implemented, it's now very easy to set the speed directly in the rig, and tlf's forcing of the speed to match the old raster just got in the way. Drop the whole idea of a list of permitted CW speeds in tlf, and allow any integer between 6 and 60 wpm. PgDn/PgUp will tune up/down in 2wpm steps. If Hamlib keying is used, and the rig exposes the range of supported wpm values, additionally honor that. (The IC-7610 does, but rigctld does not.) Since "speed" is now the plain wpm value as an integer, clean up the code and remove all occurrences of GetCWSpeed() and SetCWSpeed() to directly reference the variable. (Especially the latter was easy to mix up with setspeed() which actually *sets* the rig speed, while SetCWSpeed was just a fat setter function for the internal variable.) Drop the test code around the speed list handling since there's nothing to test anymore.
|
Thanks for the review! I swear I had read the git diff a gazillion times, yet it had escaped to me that I had mistakenly removed test/test_rules.c. Sorry, it's back now. (In my defense, running src/hamlib_keyer.c reformatted. CI is green now. |
|
Don't worry. That's what a second set of eyes are for... Thanks for the PR. |
|
Thanks! |
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 old coding had a fixed list of allowed CW speeds that were stepped through using PgDn/PgUp. While this might make sense for different step sizes (for example 2 wpm between 12 and 30, and 5 wpm above 30), it was not used as such. All steps were 2 wpm, and jumps from 12 to 6 and 50 to 60 at the very ends of the scale.
For slow speed contest training, it does make sense to allow speeds between 6 and 12 wpm. Also, since Hamlib keying was implemented, it's now very easy to set the speed directly in the rig, and tlf's forcing of the speed to match the old raster just got in the way.
Drop the whole idea of a list of permitted CW speeds in tlf, and allow any integer between 6 and 60 wpm. PgDn/PgUp will tune up/down in 2wpm steps. If Hamlib keying is used, and the rig exposes the range of supported wpm values, additionally honor that. (The IC-7610 does, but rigctld does not.)
Since "speed" is now the plain wpm value as an integer, clean up the code and remove all occurrences of GetCWSpeed() and SetCWSpeed() to directly reference the variable. (Especially the latter was easy to mix up with setspeed() which actually sets the rig speed, while SetCWSpeed was just a fat setter function for the internal variable.)
Drop the test code around the speed list handling since there's nothing to test anymore.