volume_status add wpctl support

[valdur55]

No description provided.

[valdur55]

Well. Looks ok now!

[valdur55]

Need to drop manual device setting because device ID-s are dynamic.

[lasers]

Need to drop manual device setting because device ID-s are dynamic.

device doesn't have to be number. It depends on backend.

---

According to wiki, you may be able to supply device.name or node.name to device.

Choose a property that is unique and stable for use in matches rules. The device.name or node.name properties are recommended.

Avoid using device.id, as it is dynamic and may change between sessions.

src https://wiki.archlinux.org/title/WirePlumber#Obtain_interface_name_for_rules_matching

EDIT: I assume it wouldn't work, but technically, you could create a small function that runs a command to see if that particular device name/node is active before running any other function.

[valdur55]

Added quite silly wpctl status -n parser.
Already looked into pactl devices list and found out that device name format matches,
Thankfully wpctl doesn't care about device input/output type. Control commands are same, and therefore it is possible to have faulty config:
device : output-device
is_input : TRUE
Is it worth it to prevent that happening?

Is "if self.dev_target:" guarding overkill or not? I personally don't like to send invalid shell command which might have unexpected side effects. ( sending None as device id)

In general view, ready for review.

[lasers]

therefore it is possible to have faulty config

I hope we can honor config as close as possible, especially on various backends. If user placed bad/invalid config, then I suppose we would not ignore it. However, it's up to you because...

Personally, I think/wonder if is_input config is a terrible user config that should be just removed because maybe the backends can figure is_input out themselves from device name / device information (in each setup)... We would still keep the placeholder.

Just not in this PR.

Added quite silly wpctl status -n parser.

Apparently there is pw-dump too (located in pipewire-utils). Ideal for JSON scripting. Just FYI. I don't know if it's worth it. Might be more clean/more reliable than wpctl if wpctl status -n changed one day. EDIT: Not a right tool.

[lasers]

Just wondering. Why do we need try/except with verified installed wpctl? Is this faulty None config you mentioned?

EDIT: It's for self.dev_target = None checks?

[lasers]

(Experiment/Optimization) Without testing anything... I think maybe for "performance reason".

Try noop: pass function (and real resolve_device_id function)... Assign a correct function at setup to basically make this optimized when device is not in use... and potentially avoid try/except + send None issue.

(Nit) Self.device, self.dev_target, self._resolve_device_id is blending in... Can you rename to self.target and self._resolve_target?

[lasers]

To better understand this issue, I coded too.

Please look at this. https://github.com/lasers/py3status/tree/volume_status-wpctl-wip

This is still your contribution. Take it if you find this to be better.

I noticed that...

wpctl status -n gives us same thing we need from get_volume call.... By using this data instead, we can skip extra subprocess call. One call, not two... on every refresh/volume changed, device changed, etc.

We use this output to find target_id too.
We also can use this output to find default device anyway. Not in use, but...

Microphone percentage/muted now works too... because of self._target_id.

Unrelated -- I think it would be useful in adding format and format_device and defaulting devices to (default * devices)... meaning volume_status can spawn default source+sink on the bar by default (or be silly and spawn all sources+sinks). And convert code to use thresholds too. All in a different PR though.

[valdur55]

therefore it is possible to have faulty config

I hope we can honor config as close as possible, especially on various backends. If user placed bad/invalid config, then I suppose we would not ignore it. However, it's up to you because...

My code was just PoC. Looks like your code handles SINK/SOURCE correctly.

Personally, I think/wonder if is_input config is a terrible user config that should be just removed because maybe the backends can figure is_input out themselves from device name / device information (in each setup)... We would still keep the placeholder.

We still need it to differentiate between default SINK/SOURCE

To better understand this issue, I coded too.

Please look at this. https://github.com/lasers/py3status/tree/volume_status-wpctl-wip

This is still your contribution. Take it if you find this to be better.

I tested your code. Looks like working OK; but seems like output type should be int not a float.

I noticed that...

wpctl status -n gives us same thing we need from get_volume call.... By using this data instead, we can skip extra subprocess call. One call, not two... on every refresh/volume changed, device changed, etc.

We use this output to find target_id too. We also can use this output to find default device anyway. Not in use, but...

You are right. It is possible to get needed info from status command. Nice work with parsing it!
With default device we don't need to parse status output, we can directly do get-volume command.

Microphone percentage/muted now works too... because of self._target_id.

Interesting bug. Microphone percentage/muted was working for me.

Unrelated -- I think it would be useful in adding format and format_device and defaulting devices to (default * devices)... meaning volume_status can spawn default source+sink on the bar by default (or be silly and spawn all sources+sinks). And convert code to use thresholds too. All in a different PR though.

As I see that You added some formatting to debug output. It helps to differentiate between used backends.
Idea is interesting, useful for debugging, but default behaviour is to control default SINK. Controlling source should be optional.
What I feel missing is hiding unactive source control. It feels like it can be done with format

Also we need to update documentation to list wpctl status -n command in list:
device: Device to use. Defaults value is backend dependent. "aplay -L", "pactl list sinks short", "pamixer --list-sinks" (default None)

Also I found out that you don't need set volume upper limit in your at all, because wpctl can handle limiting itself, because limit is one argument of setting volume.

BYW: Wrapping up some code changes and I will push it soon.

[valdur55]

In general it is good to go review again.

During testing, it took time to recover computer restart. For example pactl doesn't have issue.

[lasers]

Just addressing one thing immediately. My PR didn't had this fix too.

[lasers]

Use class Meta + :.0f instead... giving users int/float volume choices.

Grep update_placeholder_format on modules for an example.