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

Pwd 218 make feed use standard command line parsing #240

Closed pwdirks closed 3 years ago

pwdirks commented 3 years ago

Re-organized Feed.py to move all main code to the end of the source file; changed to use standard argparse.py with 4 (required) positional parameters and named arguments for optional arguments and all applicable config overrides.

leskerr commented 3 years ago

Patrick, I tried running one of my feeds and I got this error:

C:\Users\Les\Documents\GitHub\PyKOB>py Feed.py 102 "Welcome message, 10/15 wpm, AC" "http://kob.sdf.org/morsekob/rss/welcome.xml" 10 -c 15 --article-pause 3 --group-pause 3 -w 0
2021-01-18 00:14:28:    Starting Feed 2.0
Traceback (most recent call last):
  File "Feed.py", line 290, in <module>
    wait = float(args.wait)
TypeError: float() argument must be a string or a number, not 'list'

~Les

pwdirks commented 3 years ago

Hi Les,

I looked at the code and I see what’s causing that. I’ve fixed it in the latest commit, along with re-enabling your choice of Morse encoding with -T/--type.

Try pulling the latest commit on the branch and let me know how that works for you?

Thanks! -Patrick.

On Jan 18, 2021, at 12:19 AM, Les Kerr notifications@github.com wrote:

Patrick, I tried running one of my feeds and I got this error:

C:\Users\Les\Documents\GitHub\PyKOB>py Feed.py 102 "Welcome message, 10/15 wpm, AC" "http://kob.sdf.org/morsekob/rss/welcome.xml" 10 -c 15 --article-pause 3 --group-pause 3 -w 0 2021-01-18 00:14:28: Starting Feed 2.0 Traceback (most recent call last): File "Feed.py", line 290, in wait = float(args.wait) TypeError: float() argument must be a string or a number, not 'list' ~Les

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MorseKOB/PyKOB/pull/240#issuecomment-762071394, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALTAYK56WSPHBJBRFXASH73S2PVH7ANCNFSM4WG2RS6Q.

leskerr commented 3 years ago

Making progress, Patrick! When I tried the branch that provides an option for International Morse, I ran into two issues (probably related):

  1. The feed defaults to International, instead of American.
  2. Even when I specify the option for American, it still sends in International.

Here's the command I'm using:

C:\Users\Les\Documents\GitHub\PyKOB>py Feed.py 102 "Welcome message, 10/15 wpm, AC" "http://kob.sdf.org/morsekob/rss/welcome.xml" 10 -c 15 --article-pause 3 --group-pause 3 -w 0 -T AMERICAN
2021-01-18 09:30:27:    Starting Feed 2.0

~Les

pwdirks commented 3 years ago

Well, that won’t do on a MorseKOB wire, LOL!

What you report is very surprising. As far as I can tell parsing and creation of the Sender seem to be happening correctly. I added a “print(“Feed code type =“, code_type)” before the creation of ‘mySender’ on line 296 and it does print “Feed code type = AMERICAN”.

I can’t seem to test it by feeding a wire (I’m testing on 202) on my Mac and listen on a (Linux) VM, even though the VM hears, say, the BBC wire on 107 just fine. I’m guessing I can’t send to myself because to the MKOB server all house nodes look like my one cable modem IP address?

How do you test it?

Thanks! -Patrick.

On Jan 18, 2021, at 10:08 AM, Les Kerr notifications@github.com wrote:

Making progress, Patrick! When I tried the branch that provides an option for International Morse, I ran into two issues (probably related):

The feed defaults to International, instead of American. Even when I specify the option for American, it still sends in International. Here's the command I'm using:

C:\Users\Les\Documents\GitHub\PyKOB>py Feed.py 102 "Welcome message, 10/15 wpm, AC" "http://kob.sdf.org/morsekob/rss/welcome.xml" 10 -c 15 --article-pause 3 --group-pause 3 -w 0 -T AMERICAN 2021-01-18 09:30:27: Starting Feed 2.0 ~Les

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MorseKOB/PyKOB/pull/240#issuecomment-762401180, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALTAYK2LLR3OZXWBOVGYH4LS2R2JNANCNFSM4WG2RS6Q.

leskerr commented 3 years ago

Patrick,

At the moment, I don't see your feed on Wire 202. If you can launch it again I'd be happy to listen to it and report back.

I test by running Feed.py on my Windows laptop and listening to it by connecting to the server with MorseKOB 2.5 running on the same laptop. It works because my router automatically translates the port number in the packet coming from the server to 7890 and directs it to my laptop. This doesn't require any special configuration of the router on my part.

(I suspect the lack of a similar function on my virtual server on AWS may be why Weather.py isn't working there. But that's just speculation on my part.)

~Les

leskerr commented 3 years ago

Patrick,

I just realized why your feed on Wire 202 doesn't seem to be working. It's because the 200 series wires are private wires reserved for museums and require a PIN code to access them. Wire 202, for example is assigned to 'Wells Fargo wire 1'. You should probably move any of your feeds to the 130 series, maybe starting with 131.

@AESilky Your weather feed on Wire 206 has a similar problem. That wire is a semi-private wire, meaning anyone can listen, but a PIN is required to send. It's assigned to 'RR Museum of PA wire 1'. You could take over the 120 series and move your weather feed to wire 121 if you like.

~Les

AESilky commented 3 years ago

Ah, @leskerr , I remember you mentioning that. My weather feed is really just a test, so I can stop it. I do want to keep the Instructograph feed. I only plan to do a couple of the more interesting tapes.

I also plan to do one for the dial-up recordings. I can probably take a couple wires for those (low-speed & high-speed).

leskerr commented 3 years ago

Patrick,

I tried the latest version of this branch (commit 68ef77c). It's working well enough that it's running all my feeds on the AC server. And voila, we have Wire 108 back again, providing a feed in International Code. Thank you!

I did run into the following glitches:

  1. It didn't allow me to enter the -T parameter in lower case. I had to specify -T INTERNATIONAL. Configure.py allows me to enter all the parameters in lower case, and I've become accustomed to doing it that way.
  2. Feed.py now sounds the Morse through my computer's speakers while it's providing the feed. It didn't used to do this. Unfortunately, this is a problem when I'm also trying to listen to the feed on the same computer. Both audios come through, and it's confusing. I tried to suppress the audio from the feed by adding the -a OFF parameter, but it didn't work.
  3. When I tried to run Configure.py on the virtual 'AC' server, it failed with the following error:
    ubuntu@ip-172-26-3-177:~$ ./configure.sh
    Traceback (most recent call last):
    File "Configure.py", line 34, in <module>
    import tkinter as tk
    ModuleNotFoundError: No module named 'tkinter'

~Les

pwdirks commented 3 years ago

Hi Les,

Thanks for giving it another try. Here’s what I make of your findings:

  1. I’ll look into option parsing - case should not be a factor and that’ll need a small tweak.

  2. Along with the ability to set command-line overrides the structure of the change is such that the starting point is now the user’s configured settings. If that’s “--audio ON” to be able to listen using computer audio, that’s now also what the Feed.py will default to, which is a bit awkward. I suspect we’re going to need to provide program-specific overrides to configuration settings, and reserve the settings for use with MKOB.

You’re right, though, that overriding it explicitly on the command line should’ve worked - I’ll look into why “-a OFF” wasn’t having the desired effect.

  1. Configure.py now has a “--gui/-G” mode and that requires (potential) use of Tkinter, which means the interfaces must at least be referenced. You need to have Tkinter installed on any machine you now run Configure.py on. I thought it was part of the standard Python install but it seems to be a separate installation. Let me know if you need specific directions for some platform - I don’t know where you run the feeds.

Thanks and 73, -Patrick.

On Jan 20, 2021, at 2:15 PM, Les Kerr notifications@github.com wrote:

Patrick,

I tried the latest version of this branch (commit 68ef77c https://github.com/MorseKOB/PyKOB/commit/68ef77c982fd6b48f34eb3fa79ec5342eb683b54). It's working well enough that it's running all my feeds on the AC server. And voila, we have Wire 108 back again, providing a feed in International Code. Thank you!

I did run into the following glitches:

It didn't allow me to enter the -T parameter in lower case. I had to specify -T INTERNATIONAL. Configure.py allows me to enter all the parameters in lower case, and I've become accustomed to doing it that way. Feed.py now sounds the Morse through my computer's speakers while it's providing the feed. It didn't used to do this. Unfortunately, this is a problem when I'm also trying to listen to the feed on the same computer. Both audios come through, and it's confusing. I tried to suppress the audio from the feed by adding the -a OFF parameter, but it didn't work. When I tried to run Configure.py on the virtual 'AC' server, it failed with the following error: ubuntu@ip-172-26-3-177:~$ ./configure.sh Traceback (most recent call last): File "Configure.py", line 34, in import tkinter as tk ModuleNotFoundError: No module named 'tkinter' ~Les

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MorseKOB/PyKOB/pull/240#issuecomment-763985330, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALTAYK7IAIYFBBGZ72SUIGDS25IXDANCNFSM4WG2RS6Q.

leskerr commented 3 years ago

Configure.py now has a “--gui/-G” mode and that requires (potential) use of Tkinter, which means the interfaces must at least be referenced. You need to have Tkinter installed on any machine you now run Configure.py on. I thought it was part of the standard Python install but it seems to be a separate installation. Let me know if you need specific directions for some platform - I don’t know where you run the feeds.

Thanks and 73,

-Patrick.

Patrick,

I'm running the feeds on a virtual server instance in the Amazon cloud, where a GUI mode doesn't really apply. It was easier for me to just comment out the import tkinter as tk line from Configure.py, rather than install Tkinter. Configure.py works for me now.

~Les

pwdirks commented 3 years ago

Hi Les,

Glad it’s working but I’d recommend you install Tkinter instead of making one-off modifications to the source (not least because that’ll blow up if you ever hit “-G” or update to a more recent version).

All you need to do on Linux is

sudo apt-get install python-tk

and you should be all set. If you don’t use “-G” no GUI will be attempted anyway so there’s no danger.

Regards, -Patrick.

P.S. Even in an AWS instance I bet you’ve got VNC access to some simulated display, BTW... Nothing says you need to use it, of course.

On Jan 20, 2021, at 4:13 PM, Les Kerr notifications@github.com wrote:

Configure.py now has a “--gui/-G” mode and that requires (potential) use of Tkinter, which means the interfaces must at least be referenced. You need to have Tkinter installed on any machine you now run Configure.py on. I thought it was part of the standard Python install but it seems to be a separate installation. Let me know if you need specific directions for some platform - I don’t know where you run the feeds.

Thanks and 73,

-Patrick.

Patrick,

I'm running the feeds on a virtual server instance in the Amazon cloud, where a GUI mode doesn't really apply. It was easier for me to just comment out the import tkinter as tk line from Configure.py, rather than install Tkinter. Configure.py works for me now.

~Les

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MorseKOB/PyKOB/pull/240#issuecomment-764057278, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALTAYK4YO242K7CMD3JNMM3S25WS3ANCNFSM4WG2RS6Q.

AESilky commented 3 years ago

I think the import of tkinter needs to be wrapped in a 'try' block similar to what we do with audio. If it can't be imported then it won't be used.

This fails in the same way in my IBM Cloud VM server instance.

I can import Tkinter, but I don't think I should need to since I don't do any GUI operations on that server. It doesn't have VNC access unless I add it.

On this server, I have no need for TkInter.

pwdirks commented 3 years ago

Hi Ed,

OK, I suppose that’s reasonable; I’ve created issue #250, "Inclusion of optional libraries should be guarded with "try ... except" and the functionality disabled if not present” to cover that change and that should also fix the Configure.py issue.

73, -Patrick.

On Jan 20, 2021, at 11:11 PM, Ed Silky notifications@github.com wrote:

I think the import of tkinter needs to be wrapped in a 'try' block similar to what we do with audio. If it can't be imported then it won't be used.

This fails in the same way in my IBM Cloud VM server instance.

I can import Tkinter, but I don't think I should need to since I don't do any GUI operations on that server.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MorseKOB/PyKOB/pull/240#issuecomment-764434130, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALTAYKZ4TID6XL4HPZJJSBLS27HT7ANCNFSM4WG2RS6Q.

pwdirks commented 3 years ago

@leskerr - last call: are you ready for the command line to Feed.py to change?

leskerr commented 3 years ago

A couple of additional issues with Feed.py:

  1. When I run Feed.py on my server, each feed produces an error message like this:
2021-01-22 16:07:37 ERROR:  Wire number value '' is not a valid integer value.
invalid literal for int() with base 10: ''

The feed runs normally, however.

  1. The usage instructions and change history in the comment block at the beginning of the code are no longer current. They should either be updated or (more realistically, perhaps) removed.

~Les

pwdirks commented 3 years ago

Fixed issues noted by Les:

  1. Translated "AMERICAN" or "INTERNATIONAL" (as well as "American" or "American" etc. to proper enum values

  2. Parsed ON/OFF/YES/NO etc. as possible values for -a (audio) and set audio options as specified.

This does introduce a dependency on distutils.py in Feed.py but I believe distutils is an expected package, and doesn't need to be guarded/used only if available? (Lowercase) config.py already depends on it unconditionally.

pwdirks commented 3 years ago

Hi Ed,

That’s a fair point. I’ve left in the .upper() on the config-file value so if people DO edit the file, at least they can get away with their choice of “american”, “American”, or “AMERICAN” but I agree “I” or “A” is unnecessarily short.

I’ll fix it in the next commit.

73, -Patrick.

On Jan 22, 2021, at 11:27 PM, Ed Silky notifications@github.com wrote:

@AESilky commented on this pull request.

A question and suggestion.

In pykob/config.py https://github.com/MorseKOB/PyKOB/pull/240#discussion_r563031429:

     _code_type = (user_config.get(__CONFIG_SECTION, __key)).upper()
  • if _code_type == "AMERICAN":
  • if _code_type == "A" or _code_type == "AMERICAN": code_type = CodeType.american
  • elif _code_type == "INTERNATIONAL":
  • elif _code_type == "I" or _code_type == "INTERNATIONAL": code_type = CodeType.international Although the option processing supports 'A/a' and 'I/i', the configuration file always has 'AMERICAN' or 'INTERNATIONAL'. Are you suggesting that the configuration file also support having a value of 'A' or 'I'? I don't see the need for that. I prefer that the configuration file only have a single value. I don't really think people should be editing the configuration (soon to be 'preferences') file directly, but if they do they should be expected to use the defined property/option values.

In pykob/preferencesWindow.py https://github.com/MorseKOB/PyKOB/pull/240#discussion_r563032252:

  • if config.code_type.to_string() == self.CODE_TYPE_SETTINGS[codeTypeRadioButton]:
  • if config.code_type.to_string().upper() == self.CODE_TYPE_SETTINGS[codeTypeRadioButton].upper(): You can use config.code_type.name rather than to_string(). Likewise, config.code_type.name.upper(). It is more clear to read.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MorseKOB/PyKOB/pull/240#pullrequestreview-574743227, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALTAYK2GVGYYS5PYOIHYXSTS3J25FANCNFSM4WG2RS6Q.

pwdirks commented 3 years ago

Hi Ed,

Nice catch - that’s supposed to be “args.serial_port” and “args.interface_type”. Since there are command line arguments for each, the values should be taken from the command line (which default to whatever’s configured). Probably just cut-and-pasted from somewhere.

I’ll fix it in the next commit.

73, -Patrick.

On Jan 23, 2021, at 12:00 AM, Ed Silky notifications@github.com wrote:

@AESilky commented on this pull request.

In Feed.py https://github.com/MorseKOB/PyKOB/pull/240#discussion_r563035404:

  • Default group pause to article pause value if no group pause value is supplied

  • grpPause = args.grpPause if args.grpPause > 0.0 else args.artPause
  • Number of days (from today) of articles to read before repeating:

  • days = args.days
  • The wait time (in seconds) after someone else transmits before resuming feed:

  • wait = args.wait
  • playback_finished = threading.Event()
  • playback_last_sender = None
  • mySender = morse.Sender(wpm, cwpm, codeType=code_type)
  • myInternet = internet.Internet(idText)
  • audio_setting = strtobool(str(args.sound))
  • myKOB = kob.KOB(port=config.serial_port, interfaceType=config.interface_type, audio=audio_setting) Are serial_port and interface_type not allowed to be overridden? That seems like it causes a problem if you want to run Feed on a machine where you also want to run a client. I think this is what leads to @leskerr https://github.com/leskerr 's problem where the sounder is sounding when Feed is running. I think those need to be the same as audio_setting. Otherwise, you can't run Feed on the same machine as a client (like MKOB) if you configure a port and sounder interface. That is, of course, until multiple profiles are supported - a ways off.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MorseKOB/PyKOB/pull/240#pullrequestreview-574745251, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALTAYK563GR6NF2LIL7F6ADS3J63PANCNFSM4WG2RS6Q.

pwdirks commented 3 years ago

I've created issue #253 ("Feed.py could use an option to feed w/o waiting for a user to appear") to suggest an option to Feed.py to feed regardless of connections to a wire (perhaps automatically, if --remote is not specified).

With that, I believe the work on Feed.py to use standard config options and command line overrides is complete and I propose merging this branch sometime this afternoon to close out this issue.

AESilky commented 3 years ago

I agree that we are good for now. I think an option to 'feed' regardless of connection or listeners should be 'Feed' specific and can be implemented separately.