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

Output format in kv format #271

Closed
devnore opened this issue Feb 16, 2023 · 24 comments
Closed

Output format in kv format #271

devnore opened this issue Feb 16, 2023 · 24 comments

Comments

@devnore
Copy link

devnore commented Feb 16, 2023

Description

I would love to have the option to output in key-value format. This would make life easier when using the output for scripting and not making multiple calls to get different information.

Example (yaml):

status: "found"
model: "SteelSeries Arctis (7/Pro)"
battery_percent: "43" 

Another example could be to output in a format that could be sourced.

headsetcontrol_status:="found"
headsetcontrol_model="SteelSeries Arctis (7/Pro)"
headsetcontrol_battery_percent="43" 

Thanks for an awesome tool!

@Sapd
Copy link
Owner

Sapd commented Mar 4, 2023

It's actually a very good idea and probably much better than the short option right now.

However it would need a bit of work. I would prefer a separate file which receives an array, and which can convert this to both YAML, JSON or ENV variables depending on preference. Similar to the -o option of kubectl and most Go programs.

@devnore
Copy link
Author

devnore commented Mar 13, 2023

It's actually a very good idea and probably much better than the short option right now.

However it would need a bit of work. I would prefer a separate file which receives an array, and which can convert this to both YAML, JSON or ENV variables depending on preference. Similar to the -o option of kubectl and most Go programs.

Had the same thoughts. I was aiming to keep it as simple as possible and at the same time not introducing new problems. The only reason why I didn't give JSON as an example it that it is a PITA to write compared to YAML :)

The absolute best solution would be to have multiple formatters (JSON/YAML/TAML/INI/SH) but implementing that would require some rewrites.

@Sapd
Copy link
Owner

Sapd commented Apr 30, 2023

I took a look again at which responses we have inside the code. And actually we don't have any nested or dynamic responses, also everything is already there in main.c . That means implementing that feature (even with support of multiple languages) could be quite easy.

However, we would need to define how responses should look like. It should be simple and easy to parse.

I think at the beginning I would support YAML, Sourceable/ENV format, JSON.

Thats what I would purpose (example given in JSON):

Successful response for command

Will be responded when setting Sidetone. But also when nothing is specified, so that the program can retrieve the number of the connected headset. When nothing is specified, we could also additionally include the capabilities (see below)

{
    "status": "success",
    "device": "Corsair Void",
    "idVendor": "0x1b1c",
    "idProduct": "0x1b27",
}

Successful request

(device etc. will be also returned, just left out here for the discussion)

{
    "status": "success",
    "battery": 42
}

Also instead of the number BATTERY_CHARGING could be the status. Should we simply make it like this:

{
    "status": "success",
    "battery": "CHARGING"
}

or

{
    "status": "success",
    "batterystatus": "CHARGING"
}

and

{
    "status": "success",
    "batterystatus": "DISCHARGING"
    "battery": 42
}

Failed response for command

I am especially not sure about how to pass errors exactly to the user, here is my purpose. Everything is on one level to make it easier to parse:

{
    "status": "error",
    "errortype": "notfound", # e.g. could be notfound, timeout, parametererror, hiderror
    "errormessage": "Device not found" # Depending on what error type is, this will contain a message from headsetcontrol, or errormessage/number from HIDAPI
}

Also for battery requests, there are additionally BATTERY_UNAVAILABLE and BATTERY_CHARGING. We could also incoperate them into errortype or use batterystatus like above

Capabilities

When nothing is specified, or when explicitly requesting caps. In theory we could also return it on every command:

{
    "status": "success",
    "device": "Corsair Void",
    "idVendor": "0x1b1c",
    "idProduct": "0x1b27",
    "capabilities": ["CAP_SIDETONE", "CAP_BATTERY_STATUS"],
    "capabilities_str": ["sidetone", "battery status"]
}

For env, I would so it similar to UNIX ENVs like PATH:

HEADSETCONTROL_STATUS="success"
HEADSETCONTROL_DEVICE="SteelSeries Arctis (7/Pro)"
HEADSETCONTROL_IDVENDOR="0x1b1c"
HEADSETCONTROL_IDPRODUCT="0x1b27"
HEADSETCONTROL_CAPS="CAP_SIDETONE:CAP_BATTERY_STATUS"
HEADSETCONTROL_CAPS_STR="sidetone:battery status"

I would be happy for any feedback or suggestions.

Also tagging some people of 3rd party software @centic9 @ChrisLauinger77 @zampierilucas if some of you want to provide feedback.

@ChrisLauinger77
Copy link
Contributor

ChrisLauinger77 commented May 1, 2023

How would this be implemented ? Additionally to the existing stuff or as replacement ?
How can I find out if JSON is a supported output ?
I like the JSON example:
{
"status": "success",
"batterystatus": "DISCHARGING"
"battery": 42
}

@Sapd
Copy link
Owner

Sapd commented May 1, 2023

How would this be implemented ? Additionally to the existing stuff or as replacement ?

As an addition for now. I would deprecate the short output, but it would still be there for probably the next two years or so (maybe longer, or also shorter when most 3rd party software migrated).

The short output could receive a warning in the --help text, and probably a message via stderr when it is used (so that it does not affect current third party software).

How can I find out if JSON is a supported output ?

When this is implemented, you would simply use it like this: headsetcontrol -b -o json simply typing headsetcontrol -o would list all supported formats.

@ChrisLauinger77
Copy link
Contributor

Thx for the answers.

@Sapd
Copy link
Owner

Sapd commented Feb 21, 2024

The feature is practically finished and in the feature-output branch: https://github.com/Sapd/HeadsetControl/tree/feature-output

Here is an example when you just call -o yaml:

---
name: "HeadsetControl"
version: "2.7.0-8-ga5819e0"
api_version: "1.0"
hidapi_version: "0.13.1"
device_count: 1
devices:
  - status: "success"
    device: "HeadsetControl Test device"
    vendor: "HeadsetControl"
    product: "Test device"
    id_vendor: "0xf00b"
    id_product: "0xa00c"
    capabilities:
      - CAP_SIDETONE
      - CAP_BATTERY_STATUS
      - CAP_NOTIFICATION_SOUND
      - CAP_LIGHTS
      - CAP_INACTIVE_TIME
      - CAP_CHATMIX_STATUS
      - CAP_VOICE_PROMPTS
      - CAP_ROTATE_TO_MUTE
      - CAP_EQUALIZER_PRESET
      - CAP_EQUALIZER
      - CAP_MICROPHONE_MUTE_LED_BRIGHTNESS
      - CAP_MICROPHONE_VOLUME
    capabilities_str:
      - sidetone
      - battery
      - notification sound
      - lights
      - inactive time
      - chatmix
      - voice prompts
      - rotate to mute
      - equalizer preset
      - equalizer
      - microphone mute led brightness
      - microphone volume
    battery:
      status: "BATTERY_DISCHARGING"
      level: 64
    chatmix: 42

It will display all information, using -b or so is not needed.

If you use a command which sends something, like sidetone, it looks like this:

---
name: "HeadsetControl"
version: "2.7.0-8-ga5819e0"
api_version: "1.0"
hidapi_version: "0.13.1"
actions:
  - capability: "CAP_SIDETONE"
    device: "HeadsetControl Test device"
    status: "success"
device_count: 1
devices:
  - status: "success"
    device: "HeadsetControl Test device"
    vendor: "HeadsetControl"
    product: "Test device"
    id_vendor: "0xf00b"
    id_product: "0xa00c"
    capabilities:
      - CAP_SIDETONE
      - CAP_BATTERY_STATUS
      - CAP_NOTIFICATION_SOUND
      - CAP_LIGHTS
      - CAP_INACTIVE_TIME
      - CAP_CHATMIX_STATUS
      - CAP_VOICE_PROMPTS
      - CAP_ROTATE_TO_MUTE
      - CAP_EQUALIZER_PRESET
      - CAP_EQUALIZER
      - CAP_MICROPHONE_MUTE_LED_BRIGHTNESS
      - CAP_MICROPHONE_VOLUME
    capabilities_str:
      - sidetone
      - battery
      - notification sound
      - lights
      - inactive time
      - chatmix
      - voice prompts
      - rotate to mute
      - equalizer preset
      - equalizer
      - microphone mute led brightness
      - microphone volume
    battery:
      status: "BATTERY_DISCHARGING"
      level: 64
    chatmix: 42

Basically you have then an actions array.

This is how errors looks like. In this case -s makes an error and battery and chatmix:

---
name: "HeadsetControl"
version: "2.7.0-8-ga5819e0"
api_version: "1.0"
hidapi_version: "0.13.1"
actions:
  - capability: "CAP_SIDETONE"
    device: "HeadsetControl Test device"
    status: "failure"
    error_message: "Failed to set/request sidetone. Error: -1"
device_count: 1
devices:
  - status: "partial"
    device: "HeadsetControl Test device"
    vendor: "HeadsetControl"
    product: "Test device"
    id_vendor: "0xf00b"
    id_product: "0xa00c"
    capabilities:
      - CAP_SIDETONE
      - CAP_BATTERY_STATUS
      - CAP_NOTIFICATION_SOUND
      - CAP_LIGHTS
      - CAP_INACTIVE_TIME
      - CAP_CHATMIX_STATUS
      - CAP_VOICE_PROMPTS
      - CAP_ROTATE_TO_MUTE
      - CAP_EQUALIZER_PRESET
      - CAP_EQUALIZER
      - CAP_MICROPHONE_MUTE_LED_BRIGHTNESS
      - CAP_MICROPHONE_VOLUME
    capabilities_str:
      - sidetone
      - battery
      - notification sound
      - lights
      - inactive time
      - chatmix
      - voice prompts
      - rotate to mute
      - equalizer preset
      - equalizer
      - microphone mute led brightness
      - microphone volume
    errors:
      battery: "Error retrieving battery status"
      chatmix: "Error retrieving chatmix status"

Besides yaml, json and env is supported. Thats how ENV looks like:

HEADSETCONTROL_NAME="HeadsetControl"
HEADSETCONTROL_VERSION="2.7.0-8-ga5819e0"
HEADSETCONTROL_API_VERSION="1.0"
HEADSETCONTROL_HIDAPI_VERSION="0.13.1"
ACTION_COUNT=1
ACTION_0_CAPABILITY="CAP_SIDETONE"
ACTION_0_DEVICE="HeadsetControl Test device"
ACTION_0_STATUS="success"
DEVICE_COUNT=1
DEVICE_0="HeadsetControl Test device"
DEVICE_0_CAPABILITIES_AMOUNT=12
DEVICE_0_CAPABILITY_0="CAP_SIDETONE"
DEVICE_0_CAPABILITY_1="CAP_BATTERY_STATUS"
DEVICE_0_CAPABILITY_2="CAP_NOTIFICATION_SOUND"
DEVICE_0_CAPABILITY_3="CAP_LIGHTS"
DEVICE_0_CAPABILITY_4="CAP_INACTIVE_TIME"
DEVICE_0_CAPABILITY_5="CAP_CHATMIX_STATUS"
DEVICE_0_CAPABILITY_6="CAP_VOICE_PROMPTS"
DEVICE_0_CAPABILITY_7="CAP_ROTATE_TO_MUTE"
DEVICE_0_CAPABILITY_8="CAP_EQUALIZER_PRESET"
DEVICE_0_CAPABILITY_9="CAP_EQUALIZER"
DEVICE_0_CAPABILITY_10="CAP_MICROPHONE_MUTE_LED_BRIGHTNESS"
DEVICE_0_CAPABILITY_11="CAP_MICROPHONE_VOLUME"
DEVICE_0_BATTERY_STATUS="BATTERY_DISCHARGING"
DEVICE_0_BATTERY_LEVEL=64
DEVICE_0_CHATMIX=42
DEVICE_0_ERROR_COUNT=0

@ChrisLauinger77
Copy link
Contributor

ChrisLauinger77 commented Feb 27, 2024

I am testing the version:

headsetcontrol -?
Found SteelSeries Arctis (7/Pro)!

Capabilities:
* sidetone
* battery
* notification sound
* lights
* inactive time

with -? the chatmix is missing and also the "* battery status" is now only "* battery"
but "-o json" gives me the chatmix
the capabilities between -? and -o json do not match ? why ? I guess notification sound should be chatmix ?

headsetcontrol -o json
{
  "name": "HeadsetControl",
  "version": "2.7.0-8-ga5819e0",
  "api_version": "1.0",
  "hidapi_version": "0.14.0",
  "device_count": 1,
  "devices": [
    {
      "status": "success",
      "device": "SteelSeries Arctis (7/Pro)",
      "vendor": "SteelSeries ",
      "product": "SteelSeries Arctis 7",
      "id_vendor": "0x1038",
      "id_product": "0x12ad",
      "capabilities": [
        "CAP_SIDETONE", 
        "CAP_BATTERY_STATUS", 
        "CAP_LIGHTS", 
        "CAP_INACTIVE_TIME", 
        "CAP_CHATMIX_STATUS"
      ],
      "capabilities_str": [
        "sidetone", 
        "battery", 
        "lights", 
        "inactive time", 
        "chatmix"
      ],
      "battery": {
        "status": "BATTERY_DISCHARGING",
        "level": 0
      },
      "chatmix": 64
    }
  ]
}

@ChrisLauinger77
Copy link
Contributor

Other then -? not matching -o json it works quite well.
I updated my extension to use -o json - and when it does not work it will fall back to the old format.
I wanted to clarify the findings before I release the new version though

@Sapd
Copy link
Owner

Sapd commented Feb 27, 2024

Good catch! Will have to check it. I also noticed another bug where it will output the capabilities where it shouldnt (I think when just calling the program).

Also as side note -? will be then just meant for human output.

@Sapd
Copy link
Owner

Sapd commented Feb 27, 2024

Also as a hint, you can test also use test devices to generate test output:

./headsetcontrol --test-device 0 -o json

And this one will generate an output with errors (on purpose):

./headsetcontrol --test-device 1 -o json

@ChrisLauinger77
Copy link
Contributor

Good to know, thx.
I continue testing when the update is ready.

@ChrisLauinger77
Copy link
Contributor

ChrisLauinger77 commented Feb 29, 2024

The battery status also does not seem to work.
I get always BATTERY_DISCHARGING - even when the headset is plugged into the charger or if it is turned off.

-o json

 "battery": {
        "status": "BATTERY_DISCHARGING",
        "level": 97
      },

it should show charging - it is plugged to the USB cable which supplies power.
When it is turned off level shows 0 but still status wrong.

@Sapd
Copy link
Owner

Sapd commented Feb 29, 2024

it should show charging - it is plugged to the USB cable which supplies power. When it it turned off level shows 0 but still status wrong.

Yeah this is probably not fully implemented for your headset, that why it always show DISCHARGING, only some implementations also implement the other states.

@ChrisLauinger77
Copy link
Contributor

ChrisLauinger77 commented Feb 29, 2024

it should show charging - it is plugged to the USB cable which supplies power. When it it turned off level shows 0 but still status wrong.

Yeah this is probably not fully implemented for your headset, that why it always show DISCHARGING, only some implementations also implement the other states.

Ah I see. Maybe instead of show "BATTERY_DISCHARGING" just show "UNKNOWN" or "NOTIMPLEMENTED".
What are the possible BATTERY states anyways ?
I think it would be good to know If the BATTERY status is relyable or not.

@Sapd
Copy link
Owner

Sapd commented Feb 29, 2024

it should show charging - it is plugged to the USB cable which supplies power. When it it turned off level shows 0 but still status wrong.

Yeah this is probably not fully implemented for your headset, that why it always show DISCHARGING, only some implementations also implement the other states.

Ah I see. Maybe instead of show "BATTERY_DISCHARGING" just show "UNKNOWN" or "NOTIMPLEMENTED". What are the possible BATTERY states anyways ? I think it would be good to know If the BATTERY status is relyable or not.

Only this

enum battery_status {
    BATTERY_UNAVAILABLE = 65534,
    BATTERY_CHARGING    = 65535,
    BATTERY_AVAILABLE   = 65500,
};

(while AVAILABLE is same as DISCHARGING).

Problem is, if there are no special flags, it always assumes AVAILABLE or DISCHARGING (as long as the packet was received successfully). So there is not really a way to show unknown or something like that. One could in theory guess that when its 0, but its just an assumption.

@ChrisLauinger77
Copy link
Contributor

ChrisLauinger77 commented Feb 29, 2024

it should show charging - it is plugged to the USB cable which supplies power. When it it turned off level shows 0 but still status wrong.

Yeah this is probably not fully implemented for your headset, that why it always show DISCHARGING, only some implementations also implement the other states.

Ah I see. Maybe instead of show "BATTERY_DISCHARGING" just show "UNKNOWN" or "NOTIMPLEMENTED". What are the possible BATTERY states anyways ? I think it would be good to know If the BATTERY status is relyable or not.

Only this

enum battery_status {
    BATTERY_UNAVAILABLE = 65534,
    BATTERY_CHARGING    = 65535,
    BATTERY_AVAILABLE   = 65500,
};

(while AVAILABLE is same as DISCHARGING).

Problem is, if there are no special flags, it always assumes AVAILABLE or DISCHARGING (as long as the packet was received successfully). So there is not really a way to show unknown or something like that. One could in theory guess that when its 0, but its just an assumption.

Thanks for explaining. I think I will not show the status in my extension to not confuse the users.
It will only show discharging for some and they might file issues :(

@Sapd
Copy link
Owner

Sapd commented Feb 29, 2024

Thanks for explaining. I think I will not show the status in my extension to not confuse the users.
It will only show discharging for some and they might file issues :(

Well it is implemented in such a way that its best to basically show nothing when its DISCHARGING. But show the status UNAVAILABLE or CHARGING when this is reported, as those are only shown when they are implemented.

Will probably included it in the docs. Also another way would be renaming it to BATTERY_SUCCESS, that maybe is a bit less ambivalent

@ChrisLauinger77
Copy link
Contributor

I think "DISCHARGING" is fine.
And I can add only the charging state to my extension when the status is "BATTERY_CHARGING".
Every other state I simply do not visualize.

@ChrisLauinger77
Copy link
Contributor

ChrisLauinger77 commented Feb 29, 2024

Do you plan to release it in the near future ? version 2.8.0 ?

@Sapd
Copy link
Owner

Sapd commented Feb 29, 2024

Do you plan to release it in the near future ? version 2.8.0 ?

Yes, probably in around one month

@theaino theaino mentioned this issue Mar 16, 2024
@Sapd
Copy link
Owner

Sapd commented Mar 18, 2024

@ChrisLauinger77 I fixed the capabilities. Also I renamed BATTERY_DISCHARGING into BATTERY_AVAILABLE.
Also it can now return the battery level and BATTERY_CHARGING at the same time.

    battery:
      status: "BATTERY_CHARGING"
      level: 50

Btw I renamed battery status in just battery

@ChrisLauinger77
Copy link
Contributor

@ChrisLauinger77 I fixed the capabilities. Also I renamed BATTERY_DISCHARGING into BATTERY_AVAILABLE.

Also it can now return the battery level and BATTERY_CHARGING at the same time.

    battery:

      status: "BATTERY_CHARGING"

      level: 50

Btw I renamed battery status in just battery

Thx 4 info. My extension still works :)

@Sapd
Copy link
Owner

Sapd commented Apr 1, 2024

Completed in #333

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants