MorseKOB / PyKOB

Python implementation of a library for Morse and MorseKOB functionality
https://sites.google.com/site/morsekob/morsekob40
MIT License
4 stars 2 forks source link

MKOB - Configuration values should come from the Config object #42

Closed AESilky closed 4 years ago

AESilky commented 4 years ago

The configuration values for things like 'text speed', 'port', 'computer audio', 'code type', etc. should come from the Config object, not hard-coded.

A separate issue for saving changes to the preferences/configuration should be created.

AESilky commented 4 years ago

Configuration values are managed by MKOB/kobconfig.py. The values are all currently hard-coded. This should update this to read from the Config object. Optionally, it could be updated to incorporate support for command line options to override the saved configuration, but that should be a separate issue once this is implemented (or ahead, which this as a requirement).

pwdirks commented 4 years ago

I'm not sure it's desirable to expect 100% of user parameters to come from the config object. Perhaps for things that are configured in the first place but what about parameters unique to the specific application (e.g. "--begin-time" for Clock.py)?

Rather than rely on the config object to supply 100% of the input, why not rely on the 'args' structure to hold 100% of the parameterization? Just make sure it's pre-set to default to the config object value (e.g. for "--serial-port" and "--text-speed" etc.) and the result will be that all parameters will be captured as an 'arg' item, whether explicitly overridden by the user or unspecified and copied as-configured?

I believe the Clock.py code already has an example: pykob/config created config.PortOverride to be an argument with a default value of the configured serial port. Adding that config.PortOverride to the clock_parser provides a way for the user to specify another value. Either way, after parse_args(), port = args.Port will get you the desired serial port name?

AESilky commented 4 years ago

I wasn't suggesting that all parameters come from the config object, but all of the config properties would. The application specific properties would come from args. The application would handle processing its unique options but wouldn't handle any of the config options.

Yes, Clock and Sample are both using all of the Config argument overrides as parents to their argument processing.

As I said, I'll leave it as-is for now and we can see how it works out as we incorporate the argument processing into the other applications and support all of the config properties in the core (morse, kob, etc.).

AESilky commented 4 years ago

@leskerr This issue was actually targeted to MKOB (the GUI) and I think you implemented some of it.

Maybe we need to break this out to an issue for each parameter so they can be closed out as they are implemented?

At a minimum, I think you have (at least) partially implemented this.

Can I move it to in-progress, or maybe we can mark it done (based on the config properties you implemented) and then open new issues for the other (primary) parameters that are left to do? We can mention this issue in the new ones for reference.

leskerr commented 4 years ago

@AESilky It's probably best to leave this issue open until we've had a Zoom session to discuss how we want to handle config values. All I've been doing so far is the minimum so I can use MKOB 4.0 on the wire with other operators. The changes I've made are ad hoc and very much subject to change.

Also, I don't know what it means to move it to 'in-progress'. I defer to you and @pwdirks on how to manage the work of the project. I'm learning as we go.

pwdirks commented 4 years ago

Summarizing my take-aways from our discussion on Zoom:

  1. “Config” and “Context”/running state are very different and we shouldn’t mix the two
  2. Interaction with the config storage should be the domain of the high-level application (not any of the PyKOB functional code), which are also responsible for parsing CLI options, or interpreting GUI actions etc.
  3. The top-level application is responsible for explicitly updating the config as it sees appropriate, and for whatever parameters it deems appropriate.

I would propose that the MKOB application store the user-controllable parameters by default, immediately as they’re changed or through a separate “Preferences”/“Config” menu (default wire #, code speed, morse type, audio setting, sounder serial port) and that the CLI “utilities” (Clock.py, Feed.py, etc.) make no changes to the config but do take CLI overrides of the configured defaults. Changing the config for CLI utilities should be done through the Configure.py utility.

We talked about the possibility of storing config in the current directory, or the directory where the utility program is located, with an explicit option to change the “global” (system-wide) state instead. We may want to give some more thought to how we want multiple apps (e.g. a number of Feed.py copies running for different wires, alongside multiple copies of MKOB monitoring different wires) to be configured with their own defaults.

Regards, -Patrick.

On Jun 21, 2020, at 11:30 PM, Les Kerr notifications@github.com wrote:

@AESilky https://github.com/AESilky It's probably best to leave this issue open until we've had a Zoom session to discuss how we want to handle config values. All I've been doing so far is the minimum so I can use MKOB 4.0 on the wire with other operators. The changes I've made are ad hoc and very much subject to change.

Also, I don't know what it means to move it to 'in-progress'. I defer to you and @pwdirks https://github.com/pwdirks on how to manage the work of the project. I'm learning as we go.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/MorseKOB/PyKOB/issues/42#issuecomment-647313872, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALTAYKZDZWG3S4LWUDNI753RX33BPANCNFSM4NJKZAQA.

AESilky commented 4 years ago

On your comment:

Changing the config for CLI utilities should be done through the Configure.py utility I want to make sure it's understood that (currently) changes saved by MKOB will become the defaults used by CLI programs when they are run and changes saved by the Configure.py utility will be the settings MKOB gets the next time it is run (by the same user).

-Ed

On Sat, Jun 27, 2020 at 12:19 PM Patrick Dirks notifications@github.com wrote:

Summarizing my take-aways from our discussion on Zoom:

  1. “Config” and “Context”/running state are very different and we shouldn’t mix the two
  2. Interaction with the config storage should be the domain of the high-level application (not any of the PyKOB functional code), which are also responsible for parsing CLI options, or interpreting GUI actions etc.
  3. The top-level application is responsible for explicitly updating the config as it sees appropriate, and for whatever parameters it deems appropriate.

I would propose that the MKOB application store the user-controllable parameters by default, immediately as they’re changed or through a separate “Preferences”/“Config” menu (default wire #, code speed, morse type, audio setting, sounder serial port) and that the CLI “utilities” (Clock.py, Feed.py, etc.) make no changes to the config but do take CLI overrides of the configured defaults. Changing the config for CLI utilities should be done through the Configure.py utility.

We talked about the possibility of storing config in the current directory, or the directory where the utility program is located, with an explicit option to change the “global” (system-wide) state instead. We may want to give some more thought to how we want multiple apps (e.g. a number of Feed.py copies running for different wires, alongside multiple copies of MKOB monitoring different wires) to be configured with their own defaults.

Regards, -Patrick.

On Jun 21, 2020, at 11:30 PM, Les Kerr notifications@github.com wrote:

@AESilky https://github.com/AESilky It's probably best to leave this issue open until we've had a Zoom session to discuss how we want to handle config values. All I've been doing so far is the minimum so I can use MKOB 4.0 on the wire with other operators. The changes I've made are ad hoc and very much subject to change.

Also, I don't know what it means to move it to 'in-progress'. I defer to you and @pwdirks https://github.com/pwdirks on how to manage the work of the project. I'm learning as we go.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/MorseKOB/PyKOB/issues/42#issuecomment-647313872>, or unsubscribe < https://github.com/notifications/unsubscribe-auth/ALTAYKZDZWG3S4LWUDNI753RX33BPANCNFSM4NJKZAQA .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/MorseKOB/PyKOB/issues/42#issuecomment-650607631, or unsubscribe https://github.com/notifications/unsubscribe-auth/AETPXO5BXPSOVJAPGTVSHQTRYZA5NANCNFSM4NJKZAQA .

leskerr commented 4 years ago

On your comment: Changing the config for CLI utilities should be done through the Configure.py utility I want to make sure it's understood that (currently) changes saved by MKOB will become the defaults used by CLI programs when they are run and changes saved by the Configure.py utility will be the settings MKOB gets the next time it is run (by the same user).

That's my understanding.

leskerr commented 4 years ago

Can this issue be closed?

AESilky commented 4 years ago

MKOB and CLI apps can now get the typical configuration parameter values from the config object - so closing this issue.

As we find configuration parameters that aren't handled we will open individual issues for them.