Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pwd 16 clock announce timing #29

Merged
merged 9 commits into from
May 4, 2020
Merged

Conversation

pwdirks
Copy link
Collaborator

@pwdirks pwdirks commented May 3, 2020

This change restructures the Clock.py program to be able to announce on arbitrary intervals and in more natural formats:

"midnight"
"Nine o"clock"
"A quarter past nine"
"9:20"
"Half past nine"
"A quarter to ten"
"noon"

In the process I've created an experiment in the command line parsing, changing 'config.py' to create overrides for the options defined in the config file and changing Clock.py to builds its command line parser with the defaults as they're set in the config file but with the option to override them.

In addition I added an 'argsparser'-based command line parser for Clock-specific options:
-b <begin time>
-e <end time>
-i <interval>

The idea being you can start the clock with

python3 Clock.py -s 20 -i 15 -a OFF

to start it running at 20 WPM (regardless of configured defaults), at 15 minute intervals, with the audio always OFF, on the default configured port.

I'd welcome all comments, especially on choices of variable and global names. Existing code had some all-caps variables, and ideal choice of case for globals in 'config' is unclear to me.

Patrick Dirks added 5 commits May 2, 2020 12:42
…ed for more flexible intervals and experiment with new argument parsing
…ed for more flexible intervals and experiment with new argument parsing
…ed for more flexible intervals and experiment with new argument parsing
…ed for more flexible intervals and experiment with new argument parsing
…ce-timing

Pick up changes made in remote repository
@pwdirks
Copy link
Collaborator Author

pwdirks commented May 3, 2020

Oh, BTW - the existing code was a bit inconsistent about the 'SOUND' option - it's stored in the config file as a string ('ON' or 'OFF') but it was retrieved and used as a boolean. I believe it always came out 'True' as a result. I changed the code to retrieve it as a string and leave it that way but I had to change my code that consumed it to convert it to a boolean before creating the KOB.

I'll file a separate issue to either have all calling code configured similarly or to change the config option to save and restore a boolean. The current usage makes it awkward to set the default option for the "--sound" override.

@leskerr
Copy link
Collaborator

leskerr commented May 3, 2020

This is fantastic! I haven't tried it yet, but I love the concept from a 'learning the code' pedagogical point of view. I haven't thought through all the details yet, but by including the quarter hour text, for example, the user will learn the letter Q through subliminal reinforcement every half hour. The biggest enemy of learning the code through code practice is boredom. This way the user has a potentially useful tool (maybe as a reminder to stretch every 15 minutes while sitting in front of the computer) and at the same time a periodic refresher on code sequences that become ingrained fairly quickly.

Some users may want the option of inserting Farnsworth spacing while they're still learning.

Another useful option might be the ability to specify the text at the beginning of the announcement. It could be as simple as overriding the default 'the time is' with 'the time is now', which would give more practice with the letter W. With some imagination, the user could change the message to 'get up and stretch', or whatever. The idea is to hear a familiar phrase over and over again.

And speaking of options, what about specifying a 'quiet period'? With my original Clock program, it was hardcoded to only announce the time between 9am and 10pm, which is the way my wife wanted it. But a general program should be more flexible.

@pwdirks
Copy link
Collaborator Author

pwdirks commented May 3, 2020

Some users may want the option of inserting Farnsworth spacing while they're still learning.

I believe that will require a change in the 'kob' class, to take 2 speeds (character speed and overall speed)? When we add config options for both we can change all code to initialize the kob with the specified speeds, triggering Farnsworth spacing if requested. Of course that'll be something the user can override on the command line as well.

Another useful option might be the ability to specify the text at the beginning of the announcement. It could be as simple as overriding the default 'the time is' with 'the time is now', which would give more practice with the letter W. With some imagination, the user could change the message to 'get up and stretch', or whatever. The idea is to hear a familiar phrase over and over again.

Agreed - that'd be a nice addition. Can you file a separate issue for that request?

And speaking of options, what about specifying a 'quiet period'? With my original Clock program, it was hardcoded to only announce the time between 9am and 10pm, which is the way my wife wanted it. But a general program should be more flexible.

That's actually covered - I noticed the table had entries going from 0900 to 2200 only so I added "-b for begin" (I put the text in angle brackets and I see the comment dropped it - probably some bad XML parse in GitHub), which defaults to 900 and "-e for end" of the active period, which defaults to 2200 or 2230 (can't recall). Between "-b", "-e", and "-i" you should be able to get the announcements exactly when you want them.

@AESilky
Copy link
Collaborator

AESilky commented May 3, 2020

@pwdirks I didn't test it extensively, but for 'SOUND' I thought I used a parsing option that processed values into a boolean and accepted 'ON, TRUE, YES' as 'True' and 'OFF, FALSE, NO' as False. Did you find that to not be working correctly? I know I tested with 'ON' and 'OFF' and I believe it enabled/disabled the sound as I expected. Once I catch up on email I'll test it out again with my current installation.

@AESilky
Copy link
Collaborator

AESilky commented May 3, 2020

@pwdirks everything in the config (.ini) file is stored as a string. I believe that is a requirement of the 'configparser' class (isn't it?). But there are access methods that handle conversions.

Please let me know if that isn't the case.

@pwdirks
Copy link
Collaborator Author

pwdirks commented May 3, 2020 via email

@pwdirks
Copy link
Collaborator Author

pwdirks commented May 3, 2020

@AESilky I have no idea what I thought I was seeing - there seems to be nothing wrong with the way pykob/config.py is setting 'Sound' and it's coming out properly as a boolean. I've reverted the changes I made to that part of the config parsing logic, leaving only the command line argument overrides to be picked up by clients, which I DID fix up to convert the boolean 'Sound' back to an appropriate default ('ON' or 'OFF'). See if this isn't cleaner overall.

With the possibility of an override comes the choice to look at config.Sound (boolean) or args.Sound ('ON' or 'OFF'), defaulting to what's configured. I've change the Clock.py code to take a fresh look at the args.Sound setting and convert that back to a boolean. That'll be necessary for any program switching to using command-line overrides but for now programs using 'config.Sound' as a boolean should be just fine.

@AESilky
Copy link
Collaborator

AESilky commented May 3, 2020

I agree with @pwdirks on his earlier comment on Farnsworth spacing. That should be one of the 'common' configuration/preference options that all commands will pick up automatically by using the config module. The combination of 'speed' and 'spacing?' would set the Farnsworth value to be used. This would then apply to any program you run (Sample, Clock, News, etc.) and can be overridden on any particular instance/run of the application.

@pwdirks
Copy link
Collaborator Author

pwdirks commented May 3, 2020

Question: what would folks expect around special times (noon and midnight)? Do people say "It's a quarter to noon" or "a quarter to 12"? "quarter/half past noon" or "quarter/half past 12"?

I'm guessing at least for noon it's the latter - "noon" is only for that exact time point - otherwise it's "quarter to twelve". But what about midnight? Nobody says "quarter to zero"? Or "quarter past zero" - it's got to be "a quarter to midnight"? "Half past midnight"?

I took a survey (n=1; I asked my wife) and she felt it would be more natural around midnight to say "quarter to twelve"/"quarter past twelve" etc. So I think I'll keep "noon" and "midnight" only for those exact time points and use "twelve" otherwise.

Thoughts?

@leskerr
Copy link
Collaborator

leskerr commented May 3, 2020

My wife and I both would say "quarter to 12" and "quarter/half past 12". So that's n=3 for your survey. :-)

@AESilky
Copy link
Collaborator

AESilky commented May 3, 2020

My wife and I concur - so now you have n=5. :-)

Clock.py Outdated Show resolved Hide resolved
Clock.py Outdated Show resolved Hide resolved
Clock.py Show resolved Hide resolved
Copy link
Collaborator

@AESilky AESilky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put in a few review comments in the code. Please take a look.

…or simpler and potentially quieter debugging
@pwdirks
Copy link
Collaborator Author

pwdirks commented May 4, 2020

Les, I believe I've addressed everything you mentioned above - please have another look and I'll merge in this version. You're welcome to tweak it further as you see fit then?

@leskerr
Copy link
Collaborator

leskerr commented May 4, 2020

As usual, I can't keep up with you guys, and I don't want you to slow down on my account. Seriously...I mean it!

I have the latest version of Clock.py (I think!) running on my RPi. I have it set to announce the time on the quarter hour, and I love it! At some point I want to read through the code in detail and learn from the changes, especially the handling of parameters.

We talked a bit about the possible advantage for some users to specify Farnsworth timing for the Clock program. I think it makes more sense for the sender class in pykob/morse to default to the config settings for FW_CHAR_SPEED and FW_SPACING, if these aren't specified when a sender class is created. The user can set these parameters once using Configure.py, and the values will apply to all programs that use the code sender. I think there will rarely, if ever, be a need to do an application-specific override. I'll submit an Issue for this.

@pwdirks
Copy link
Collaborator Author

pwdirks commented May 4, 2020 via email

@AESilky AESilky self-requested a review May 4, 2020 04:43
Copy link
Collaborator

@AESilky AESilky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@pwdirks pwdirks merged commit d48ef48 into master May 4, 2020
@pwdirks pwdirks deleted the pwd-16-Clock-announce-timing branch May 4, 2020 05:00
@pwdirks
Copy link
Collaborator Author

pwdirks commented May 4, 2020

Thanks for the review!

@pwdirks pwdirks restored the pwd-16-Clock-announce-timing branch May 4, 2020 05:01
@pwdirks pwdirks deleted the pwd-16-Clock-announce-timing branch May 4, 2020 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants