Rework command line arguments

[Dinsmoor]

I am writing this to get other's opinions on reworking the invocation syntax.

The Problem

I find that a lot of people make a mistake when invoking ardopcf, and it usually comes down to the way positional arguments are required.

Currently it is invoked like ardopcf port [capturedevice playbackdevice] [Options]

What many users (including myself when I first started using this) end up doing is ./ardopcf 8515 plughw:1,0 which, on ALSA, will result in an error like:

Opening Playback Device ARDOP Rate 12000
ALSA lib pcm.c:2664:(snd_pcm_open_noupdate) Unknown PCM ARDOP
cannot open playback audio device ARDOP (No such file or directory)
Error in InitSound().  Stopping ardop.

And that is confusing, if you don't know that internally ardopcf tries to access a slave device the user might create called ARDOP in a user's ~./asoundrc file, like pcm.ARDOP {type rate slave {pcm "hw:1,0" rate 48000}} (ardopcf might want to default to the pcm.default, or make an educated guess if pcm.ARDOP is not available anyway, but that is a separate issue)

Additionally, if you intend to leave the ardopcf port 8515 the default (but need to specify a set of audio devices), why require it every single time you need to invoke the program? The existing naming conventions for arguments are inconsistent and not readily understood.

The existing --help argument displays this:

Optional Paramters
-l path or --logdir path             Path for log files
-H string or --hostcommands string   String of semicolon separated host commands to apply
                                       to ardopcf during startup, as if they had come from
                                       a connected host.  Since this duplicates the
                                       functionality of several existing optional parameters,
                                       they are marked below as (D) for deprecated.
                                       This indicates that they will be removed in a future
                                       release of ardopcf, and that their use should immediately
                                       be discontinued.  Information about replacement host
                                       commands is given for all deprecated parameters.
(D) -v path or --verboselog val      Increase (decr for val<0) file log level from default.
-----> Use -H "LOGLEVEL #" instead, where # is an integer from 0 to 8.
(D) -V path or --verboseconsole val  Increase (decr for val<0) console log level from default.
-----> Use -H "CONSOLELOG #" instead, where # is an integer from 0 to 8.
-c device or --cat device            Device to use for CAT Control
-p device or --ptt device            Device to use for PTT control using RTS
                                     or CM108-like Device to use for PTT
-g [Pin]                             GPIO pin to use for PTT (ARM Only)
                                     Default 17. use -Pin to invert PTT state
-k string or --keystring string      String (In HEX) to send to the radio to key PTT
-u string or --unkeystring string    String (In HEX) to send to the radio to unkey PTT
-L use Left Channel of Soundcard for receive in stereo mode
-R use Right Channel of Soundcard for receive in stereo mode
-e val or --extradelay val           Extend no response timeout for use on paths with long delay
(D) -x val or --leaderlength val     Sets Leader Length (mS)
-----> Use -H "LEADER #" instead, where # is an integer in the range or 120 to 2500 (mS)
(D) -t val or --trailerlength val    Sets Trailer Length (mS)
-----> Use -H "TRAILER #" instead, where # is an integer in the range or 0 to 200 (mS)
-G port or --webgui port             Enable WebGui and specify port number.
(D) -r or --receiveonly              Start in RXO (receive only) mode.
-----> Use -H "PROTOCOLMODE RXO" instead
-w or --writewav                     Write WAV files of received audio for debugging.
-T or --writetxwav                   Write WAV files of sent audio for debugging.
-d pathname or --decodewav pathname  Pathname of WAV file to decode instead of listening.
(D) -n or --twotone                  Send a 5 second two tone signal and exit.
-----> Use -H "TWOTONETEST;CLOSE" instead
-s or --sdft                         Use the alternative Sliding DFT based 4FSK decoder.
-A or --ignorealsaerror              Ignore ALSA config error that causes timing error.
                                       DO NOT use -A option except for testing/debugging,
                                       or if ardopcf fails to run and suggests trying this.

Proposed Solution

Regarding: https://github.com/pflarue/ardop/blob/ea9d0f65a2809f1b2de02b3966c356fe4d450bb4/ARDOPCommonCode/ARDOPCommon.c#L250

I propose eliminating mandatory positional arguments altogether, reduce the number of short arguments, and make some long arguments more verbose. This will make the program easier to use and save a lot of people's time trying to explain the quirks of invoking ardopcf.

Normal Argument Conventions

1. POSITIONAL arguments should be used for information that the program normally can't work right without, such as an audio device in this case.
2. SHORT arguments should really only be used for the most common of tasks, and always have a corresponding LONG alternate.
3. LONG arguments should be available for everything, and be verbose/clear in their function.

The only [optional] positional argument should be an audio device to be used for both playback and capture, because unless it is specified, ardopcf will currently default to pcm.ARDOP - invalid in most default configurations. It should also be available as a named argument in case the user prefers to be extra verbose/invoke ardopcf in a script.

Final arguments

I think, for readability and consistency, invoking ardopcf should look like this:

Usage: ardopcf [AUDIO-DEVICE] [options]

Optional Parameters
--help                             Display this message.

--help-hostcommands                Display information about valid host commands.

-a, --audio-device DEVICE         Use specified audio device for receive and transmit.                    

-p, --playback-device DEVICE      Use specified audio device for transmit.                

-c, --capture-device DEVICE       Use specified audio device for receive.

--host-port PORT                   Assigns host interface command+data ports
                                   Host interface data port = PORT+1
                                   Default: 8515

-G, --web-gui [PORT]               Enable http WebGui on specified tcp port.
                                   Default port: 8517

--log-dir PATH                     Path for log files
                                   Default: current directory

-H, --host-commands COMMANDS       String of semicolon separated host commands
                                   to apply to ardopcf during startup. See
                                   --help-hostcommands for more information.

--cat-device DEVICE                Device to use for CAT PTT control.

--ptt-device DEVICE                Device to use for PTT control using RTS
                                   keying, such as with a Digirig.

--cat-ptt-key-string HEX           String (In HEX) to send to the radio to
                                   key PTT using CAT.

--cat-ptt-unkey-string HEX         String (In HEX) to send to the radio to 
                                   unkey PTT using CAT.

--rx-left-channel                  Use only Left Channel of stereo audio
                                   device for receive.

--rx-right-channel                 Use only Right Channel of stereo audio
                                   device for receive.

--extra-timeout-delay MS           Extend ARQ no response timeout for use on
                                   paths with long delay.

--wav-write-rx [PATH]              Write WAV files of received audio to disk. 
                                   Format: 12k S16????
                                   Default path: current directory

--wav-write-tx [PATH]              Write WAV files of transmitted audio to disk.
                                   Format: 12k S16????
                                   Default path: current directory

--wav-read FILE                    Decode a wave file and exit.

--sliding-4fsk                     Use experimental Sliding DFT based 4FSK decoder.

--ignore-alsa-error                Ignore ALSA config timing error fix.
                                   Read documentation before using.

Users affected

This change would obviously affect everyone. Usage changes like this often most impacts those who invoke them via scripts. On Linux, I am not aware of any auto-start scripts for software like PAT. Users who download a new version would of course need to read the change log.

I think, that although it is different, it would be a welcome change that would make it MUCH easier to start and run ardopcf. That was my biggest frustration as a new ardopcf user.

I look forward to anyone's feedback, before I start working on implementing this in a pull request.

I think the only change from my proposed set of options, is that verbosity flags should be added back, however in implementation, they should just add the relevant host commands to --host-commands.

[pflarue]

I am not fundamentally opposed to changing some of this, but I don't want to do it more than once, so we need to think about it carefully before changing it.

I assume that most users will set up a script or bat/cmd file to start ardopcf. Thus, getting the command line options right only has to happen once for most users. Thus, it need not be particularly convenient.

Better error messages in response to inappropriate command line arguments is probably a good idea to help new users who are getting them wrong.

Currently, I don't believe that ardopcf reverts to pcm.default if PCM.ARDOP is not found, nor do I think it would necessarily be a good idea to do so. Many (most?) users use a USB sound device to interface with a radio. This is often not going to be the machine's default sound device. Thus, I think that requiring an explicit choice of sound devices is appropriate.

Since few users will choose anything other than the default port number, perhaps that should be an optional argument.

I'm willing to consider allowing a single sound device name to be used for both the playback and capture devices.

(The following paragraph is no longer relevant based on Dinsmoor's revised proposal)

Regarding your proposed new arguments, short (single dash) arguments must be only a single letter. This is how getopt() works, and it is thus common behavior of many programs. Your two letter options -ad would be interpreted as equivalent to -a -d if the -a option does not take an argument else d is taken to be the argument for the -a option.

My personal preference, though I am open to arguments to the contrary is that all options have a long form proceeded by two dashes and that most (if not all) also have a single letter from. I also don't see sufficient value in --dash-separated-forms over the current --nondashseparatedforms to change these without an additional reason to do so.

[Dinsmoor]

Once again, my naivety. :smiling_face_with_tear: I've edited the OP to fix those couple errors, and include the current --help content.

For dash separated forms, they're simply more readable.

I'll post to the ardop users group for input.

[pflarue]

Since you added a copy of the current help text, which looks really ugly compared to your proposal, I'll also mention that cleaning up the help text can, and perhaps should, be done even if the changes in functionality that you are proposing aren't implemented.

[spudmusket]

From the above:

--dash-separated-forms <- This is the "standard" UNIX way, so I would suggest it is the "correct" way. To confirm, review some simple commands i.e. ls --help, fldigi --help, even pat --help.

Having a single letter variant is also what is expected for the most common/required flags. So I support the direction outlined above as the "right" way to go.

Additional:

I suggest that a configuration file should be considered at the same time, which would allow all of the flags (long variants without preceding dashes) to be set. This would be best placed in ~/.config/ardopcf/ardopcf.conf.

An additional flag is required to be included to allow for the path of the configuration file to be set i.e. --config-file PATH - this allows multiple configuration files to be used if needed either for multiple use cases or simply debugging.

Note:

Creating a full, well documented configuration file, would be a great way to document the changes as outlined above - effectively a template for the changes prior to implementation.

Myles.

[pflarue]

I don't disagree that --dash-separated-options are better/more correct, at least on a POSIX system. However, I'm not convinced that the improvement is worth breaking existing usage and deviating from usage of (pi)ardopc. Note that my recent release deprecated (but didn't yet break) use of some existing command line arguments because of this same concern.

For normal use, most people probably won't specify very many command line options, so I'm not sure that parsing a configuration file is worth the effort. For anyone that regularly switches between a couple of configurations, creating multiple script/bat/cmd files provides an easy workaround. An advantage of these over a configuration file is that the user can decide where to put it and what to call it, without ardopcf having to deal with operating system dependent differences.

[spudmusket]

Perhaps considering the 'bigger' picture of the potential to integrating ardopcf into the likes of Pat, would see the benefits of a configuration file i.e. allowing changes to be made without the need to think about systemd configurations etc. Unless the future of ardopcf is always standalone?

Perhaps a quick look at direwolf - a soundcard modem, would be beneficial.

Just suggestions since a one off change looks to be of value to 'tidyup', and doing it with full consideration seems the right thing to do.

[Dinsmoor]

Considering a configuration file would be a separate topic, this is mainly just for improving the syntax of invoking adopcf.

@pflarue I'm not certain how this would affect the pi, because ardopcf compiles on the pi. Do you mean causing confusion with the various guides written on the internet, like this: https://la4tta.blogspot.com/2018/04/setting-up-ardop-on-raspberry-pi.html ?

I have yet to use ardopcf on the pi at all - so I'm not sure what you mean here :)

[pflarue]

Sorry for my lack of clarity. When I write "(pi)ardopc" I'm referring to John Wiseman's "ardopc", which he names "piardopc" when compiled for raspberry pi.

My concern is that someone who has been using (pi)ardopc, and replaces it with ardopcf for its additional features or bug fixes, would ideally not have to change much other than the name of the executable.

Also, a variety of Internet sites provide guidance related to configuring (pi)ardopc. Until clear, well organized, and authoritative documentation for ardopcf is readily available, people wanting to use ardopcf are likely to be using those sites as references. So, significant changes in the command line options would make it harder for such people to get ardopcf working.

Notice how ardopcf v1.0.4.1.2, which deprecates some older command line options, explicitly tells users attempting to use those options what to use instead. This approach is less practical for a complete rework. The hope here is that someone upgrading to this version will see the error messages and be able to figure out what to change, even if they copied their current options from somewhere and don't clearly understand what each option is intended to do. Also, by deprecating these options now, but not disabling them until a later release, an upgrade will print errors but still work, allowing a user to continue using Ardop without having to immediately figure out what should be changed.

[spudmusket]

Considering a configuration file would be a separate topic, this is mainly just for improving the syntax of invoking adopcf.

I disagree - if improving/changing syntax of command line options is being considered it is beneficial to look at a configuration file at the same time (much less work) and it requires a command line flag as previously indicated. There may be some command line options that are not required i.e. only configurable from a configuration file.

Perhaps a bit of a plan of where ardopcf is heading i.e. a road map, would be beneficial in plotting out what will likely be useful in the future? From my experience, a configuration file, once implemented, makes it much easier to add (and remove) lesser used options or test options i.e. no change to the command line is required, only additions/subtractions from the configuration file which has much less restrictive input than the command line...

[Dinsmoor]

I see what you both mean.

Regardless, there's no rush on the command line syntax reworking, and that can absolutely wait until ardopcf itself is done with major refactoring and documentation has started to be written. I just wanted to start the conversation ahead of time, as it's something we can think about and already have an outline for when the time arises.