Config module should track configuration of additional configuration options

[pwdirks]

There are more configuration settings than the current PORT, SOUND, and WPM options. We might also enable configuration of

STATION - local station call WIRE - default wire to connect to CWPM - received speed? (although we should probably auto-detect speed instead) LOCAL - local copy of generated traffic (using settings of SOUND and PORT) REMOTE - remote copy of generated traffic (using setting of WIRE)

[AESilky]

I will defer to you and Les on additional settings. I am a rookie/newbie to the MorseKOB. Adding additional parameters/configuration-settings isn't hard (picking '-' and '--' values for the CLI is probably the hardest part.

[AESilky]

@pwdirks, @leskerr agreed with you for the most part. To start I will add the ones indicated.

[leskerr]

CWPM is the speed that individual characters are sent at. WPM is the overall speed of the sent text. If CWPM is greater than WPM, the difference is made up by inserting additional space between the characters (Farnsworth spacing).

Optionally, the extra space can be inserted between words, in which case the whole word is sent at the full CWPM speed. Whether Farnsworth spacing is applied between characters or words is determined by the SPACING parameter, whose value can be CHARSPACING (default) or WORDSPACING. SPACING should be a configuration option.

I'm not hung up on the exact nomenclature for these parameters. I defer to @pwdirks and @AESilky to choose names that are stylistically compatible with preferred programming norms.

At some point, the MKOB program will require a MODE configuration option to accommodate different interface choices. Details are yet to be determined.

[pwdirks]

The Wikipedia article on Morse explains Farnsworth timing - see https://en.wikipedia.org/wiki/Morse_code#Farnsworth_speed for their format. It speaks of "character speed" and "text speed", the former typically higher than the latter in the Farnsworth method. http://www.arrl.org/files/file/Technology/x9004008.pdf is a write-up of an ARRL proposal for standard nomenclature - they speak of "character speed" and "overall speed".

It seems "character speed" is common enough and generally understood to be the (possibly faster) rate at which individual characters are sent. So let's use "CHAR_SPEED" or "CHARACTER_SPEED" or "CSPEED" for that one, and keep "WPM" for the overall send rate? So by default "CHAR_SPEED" would be equal to WPM but when using Farnsworth, CHAR_SPEED would be raised for the same WPM (or WPM lowered for the same CHAR_SPEED)?

[leskerr]

Both of the links mentioned by @pwdirks were new to me. The Wikipedia article on Farnsworth is excellent, and the ARRL document is even better (in the sense of being more complete).

I agree that "character speed" is a good name for that parameter. Any of the suggested spellings of the option is fine with me.

As a practical matter, character speed could default to 18 wpm and would rarely, if ever, be changed by anyone. I'm not sure it even needs to be a configuration parameter, if we want to keep the complexity of the system to a minimum. No doubt someone will ask for it sooner or later, though...maybe even me!

[pwdirks]

I suppose if someone changes the WPM configuration, should CHAR_SPEED (I don't mind CWPM, either, actually) be updated to match? Or default to 18 or WPM, whichever is higher? Or stay unchanged and change only when expliclicitly set?

I have a feeling not changing CHAR_SPEED to match WPM if the latter's being set w/o setting the former may introduce unexpected inconsistencies. If I say only "WPM=20" I'd expect characters to come out at 20 WPM. If I set WPM=10 I'd still be surprised if individual letters go "whizzing" by at either the old rate or some new fixed rate like 18 WPM. If I set WPM=5 and CHAR_SPEED=20, only THEN do I expect different spacing?

[leskerr]

The way the cwpm parameter works in pykob's code sender is that it establishes the minimum character speed, not necessarily the actual character speed — the difference being that if wpm > cwpm, then the actual character speed is wpm, not cwpm. So cwpm needs to be thought of as a 'Farnsworth setpoint', not a mandatory character speed.

Whether the user is surprised or not by what happens when they request a code speed lower than the Farnsworth setpoint depends on what their expectations are. If they understand how Farnsworth works, then they'll get what they expect. If they're expecting the character speed to follow the code speed, even at lower speeds, then indeed they'll be surprised.

I still think the cwpm parameter shouldn't change when wpm changes, but we need to make sure the program's behavior is intuitive and predictable. Perhaps the character speed parameter should be called the Farnsworth speed, and the 'spacing' parameter be called (or at least thought of as) 'Farnsworth spacing', with possible values NONE, CHARACTER, and WORD. I'd vote for CHARACTER to be the default, but that's just me.

[AESilky]

For the 'config' module, it doesn't care how the values are used only what the names are, the data type, and if there is a constraint on the valid values.

Given that - I'll go ahead and take a shot at it, but I will request reviews before merging it into master.

[AESilky]

It is pretty easy to change option and parameter names.

[AESilky]

Started implementation.

[AESilky]

From the previous comments I am implementing the following (can be modified before the branch is merged to master). It seems that some of the comments/questions about the values are on how to use them which isn't important to the config module. It only needs to know what to call it and what the data type is.

STATION - String: used for local station call WIRE - String: default wire to connect to FW_CHAR_SPEED = - Integer: character speed (if greater than wpm introduces Farnsworth timing. If less than wpm it is ignored and wpm is used) FW_SPACING - List['NONE', 'CHAR', 'WORD'] - the type of space to incorporate Farnsworth timing to LOCAL - (@pwdirks is this a path were it should be stored or a boolean indicating that it should be stored?): local copy of generated traffic (using settings of SOUND and PORT) REMOTE - (@pwdirks is this a path were it should be stored or a boolean indicating that it should be stored?): remote copy of generated traffic (using setting of WIRE)

[leskerr]

The description of FW_CHAR_SPEED might be better if it said 'Integer: Farnsworth character speed (in effect if Farnsworth timing is enabled)'. Also, there's a spurious equal sign after FW_CHAR_SPEED. (See, I can be a nitpicker too!)

I may have missed the discussion about LOCAL and REMOTE. I don't know what they're for.

Other than that, the names you've chosen are fine with me.

[pwdirks]

Sorry "I meant 'copy' in the radio sense". IOW "LOCAL" and "REMOTE" are booleans; if "LOCAL" is set your computer sound + serial will see the generated traffic; if false, the program quietly generates whatever it's generating. REMOTE is a control on whether or not any traffic is sent on the specified wire. You might test a program with LOCAL=true and REMOTE=false; you might leave it running later with REMOTE=true and LOCAL=false to get some peace and quiet :-)

[leskerr]

Ah, now I understand. I'm often in the situation @pwdirks describes, where I want to test locally (with computer audio or attached sounder) and then later send to a wire (with or without local sound, but usually without). So the requirement makes perfect sense.

Just as a thought, would it be simpler, instead of having the REMOTE option, to have WIRE = 0 imply nothing is sent remotely? This is how pykob works right now.

[pwdirks]

Well, I might still want to have "WIRE" configured to default to my favorite wire without having to force it to zero but I suppose if I could override it on the generator program's invocation only, that'd be exactly like specifying "REMOTE=OFF". Note sure why I'd want to mix that semantic into the "WIRE" setting, though? And it doesn't obviate the need for "LOCAL=OFF" operation to quietly generate traffic on some wire (e.g. newswire)? If you've got a "LOCAL" setting I think a matching "REMOTE" config option makes more sense.

[leskerr]

Yep, I was just checking to make sure the issue was well thought out. I support your original proposal. It works for me.

[AESilky]

I forgot to add "LOCAL" and "REMOTE". I'll add those and update the pull request.

[AESilky]

@leskerr and @pwdirks added a number of comments to #39 that also apply to this issue. I believe they are appropriate for the review request I generated for my pull request for this issue.

I am copying them here from #39 to have them available for this issue. I will leave them in #39 as they will be useful there as well.

@leskerr wrote:

1. I'm very happy that you're adding the character words per minute to the core. By this, I assume pykob.morse will be modified to get this parameter from the config file, unless it's overridden with an explicit parameter when a code sender instance is created. The same goes for the overall code speed parameter, of course.
2. The spacing parameter should allow for NONE (i.e. no Farnsworth spacing) in addition to CHARACTER and WORD.
3. Sooner or later we'll need a config parameter for the code type (AMERICAN or INTERNATIONAL).
4. I'm struggling to get comfortable with the term word speed. It would be unfamiliar and probably confusing to most users. I'd prefer code speed (the commonly used term) or text speed (used in the Wikipedia article on Farnsworth spacing).

I worry "code speed" will still be confusing if it's NOT intended to be the speed at which the morse code characters are sent. "Text Speed" seems clear enough, although I'm not thrilled with the term. "Message speed"? "Overall speed"?

Yeah, it seems to me we're overthinking this by trying to make the definition precise and muddying the issue for most users in the process. I took a look at the W1AW code practice page to see what language they use. The term WPM appears 40 times on that page with the implied meaning of code/text/message speed, the term speed by itself (with the same meaning) appears twice, and the term character speed is used once to distinguish it from 'normal speed'.

There's no explicit mention of Farnsworth on the W1AW page. It's just their policy to always send characters at a minimum speed of 18 wpm. I think PyKOB should do the same thing, while allowing the option to override it. We should follow the principle of making the normal things simple and the rare things possible.

The Morse News program uses the terms code speed and char speed. These make sense to me.

@pwdirks wrote:
I see "transmission speed", "word speed", and "WPM speed" used. How about if we use, as the original parameters did, just use "WPM" for the overall transmission speed and "CSPEED" or "character speed" for the speed at which individual characters are sent (still specified in the same wpm units), with a FARNSWORTH or SPACING option to specify whether to insert space between characters or between words to make the overall transmission speed (WPM) be as specified?

As for how this should be specified, I believe we're settling on a model where there's a "config" that has defaults for all values, but individual programs can override those defaults as appropriate. I agree the "default default" or ("default configuration") for CHAR_SPEED should be MAX(18, WPM), with WPM defaulting to, oh, 18? If you configure WPM to be, say, 25 you get CHAR_SPEED=25; if you configure WPM to be 10 you get "10/18" timing as the ARRL page would describe it: 10 WPM overall, with characters sent at 18 WPM rate?

[leskerr]

Keep in mind that the CHAR_SPEED parameter specifies the minimum character speed, not the actual character speed. The code sender would send characters at MAX(WPM, CHAR_SPEED), but the value of the CHAR_SPEED parameter itself wouldn't change when WPM changes. If that's confusing, then we could call it FARNSWORTH_SPEED or MIN_CHAR_SPEED instead of CHAR_SPEED.

[AESilky]

First, I want to make sure I'm correct in thinking that many of these comments are in response to the review request. I appreciate all of the feedback. There is a way to make them part of the review rather than simply comments. I'll find the specific steps for that and let you know.

Review response: To @leskerr

1. For this issue (#12) I am only adding in the ability to support specifying and optionally saving the values of the parameters and then making the values available from the config module. What I'm checking in for this issue will not actually implement the functionality within the core. I could have combined them into a single issue, but that makes the issue larger and harder for multiple people to work on. What I started doing was to then create a separate issue for implementing the functionality for each of these individual options. That way, anyone can pick up an individual one and implement it without needing to implement all of them at once. Hopefully that approach is acceptable.
2. I can easily add 'NONE' as on option, but it doesn't seem necessary to me - maybe I'm misunderstanding something, but if WPM == CWPM then there isn't any Farnsworth spacing. If they are not equal, then either CHAR or WORD would be used.
3. I mentioned that I forgot to add LOCAL and REMOTE and plan to do that later today/tonight. I'll add code_type with an enum of AMERICAN or INTERNATIONAL at the same time.

For the forth, I will combine @leskerr and @pwdirks:
I am the newbie, so please take my comments on this with that in mind...
I can change it to text_speed or code_speed, those both make sense to me. I don't care for wpm as the name of the parameter because to me that is the units being used, but I defer to you if that seems better. I also don't like cwpm for a similar reason, but again I defer to you. Given Les' additional comments/explanation I think I like min_char_speed.
So, how about: text_speed and min_char_speed?

@pwdirks description of how the config is meant to work is what I had in mind as well. I'm glad he mentioned it, because it reminded me that I didn't put defaults in for all of the new parameters I added. I will do that.

When I've added the missing parameters, changed some names, and added defaults I will send out another review request.

[AESilky]

Also, (again, take this as being from a newbie) I don't care for farnsworth_speed. In using MorseKOB I saw 'Farnsworth Delay and ms` and never really knew what good values would be to enter and what effect it would really have. If it was 'Text Speed' (or 'Code Speed'), 'Minimum Character Speed', and 'Farnsworth Spacing' I would have figured it out easily (for example, set a min_char_speed of 20, a text_speed of 13 and spacing of CHAR).

[leskerr]

The Farnsworth Delay and ms settings in MorseKOB 2.5 were a bad idea from the get-go. I'm glad 'Text Speed', 'Minimum Character Speed', and 'Farnsworth Spacing' are intuitive for you.

As for the SPACING = NONE option, that was just to provide a convenient way of turning Farnsworth on and off. Other options are to have a separate FARNSWORTH = ON or OFF parameter, or have the user set MIN_CHAR_SPEED to a low value (like 1) to turn Farnsworth off, and then set MIN_CHAR_SPEED back to what it used to be to turn Farnsworth back on again.

[AESilky]

Implementing @leskerr suggestion that the '-i/--info' option in Configure should include the:

1. Python version
2. PyKOB version
3. PyAudio version
4. PySerial version

Preparing to check in... this is the output:

1. When PyAudio and PySerial are not installed:
   PyKOB: 1.2.1 Python: 3.7.7 PyAudio: PyAudio is not installed or the version information is not available (check installation) PySerial: PySerial is not installed or the version information is not available (check installation)
2. When they are installed:
   PyKOB: 1.2.1 Python: 3.7.7 PyAudio: 0.2.11 PySerial: 3.4

I will add this to the commit/push.

[AESilky]

I updated the help in Clock.py to resolve #34 since the change for this issue was updating the help text anyway.

When this change is merged I will close #34 along with this (#12).

[AESilky]

Merged into master. Closing issue.

[AESilky]

BTW - This changed a number of the keys used in the INI file for storing the user preferences. Suggest deleting the config-<user>.ini file and then using configure to set your preferences again.

To find the configuration file run configure -i. The file paths are listed in the system information section.