Sapd / HeadsetControl

Sidetone and Battery status for Logitech G930, G533, G633, G933 SteelSeries Arctis 7/PRO 2019 and Corsair VOID (Pro) in Linux and MacOSX
GNU General Public License v3.0
1.44k stars 176 forks source link

[minor] Invalid option produces extra output "Invalid argument ?" #85

Closed VTimofeenko closed 2 years ago

VTimofeenko commented 4 years ago

Calling headsetcontrol with an invalid option (e.g. -v) produces seemingly unnecessary line "Invalid argument ?":

~ $ headsetcontrol -v
headsetcontrol: invalid option -- 'v'
Invalid argument ?
VTimofeenko commented 4 years ago

@Sapd , have you considered alternative implementation to the current commandline switches, in form of headsetcontrol {COMMAND} {VALUE} [PARAMS] like so:

image

?

From the interface perspective, I think it will make integrating something like #60 easier.

I am willing to do the legwork to start implementing this(although my C knowledge is very subpar). For now without making extra functionality to the utility, just adding alternative interface to the existing one.

Sapd commented 4 years ago

However, some questions arise regarding the form. For example, how do you combine several parameters?

The current form is the form used by almost all Unix programs. After all, Unix has support for it (via getopt).

Why does it make #60 exactly easier? (did you cite the wrong issue id?)

However, the main.c actually needs to be revised. As it contains a lot of code duplication.

VTimofeenko commented 4 years ago

combine several parameters Hm. I initially did not think about that use case, but this still could work like:

# headsetcontrol set sidetone 100 led 1

That would however break the ability to do something like

# headsetcontrol -s 100 -b

Why does it make #60 exactly easier I had a thought that this approach would allow to easier separate the battery charge from whether the headset is charging or not, like so:

# headsetcontrol get is_connected 
1
# headsetcontrol get charge
75%
# headsetcontrol get is_charging
1
Sapd commented 4 years ago

This would be theoretically possible with like this -b --charging

I don't think that's really worth changing. It would make the interface a bit simpler, but it would increase the code complexity (which is already quite complex - it is already difficult to keep it clean) and implement things that are already in the standard library.

Sapd commented 2 years ago

The interface change a bit, it is at least possible to use now long options