wip centralised thresholds system

[JakeStanger]

Overview

This is a first draft at attempting to introduce a standardised system for configuring thresholds, ie for icon boundaries.

The system would apply to the following modules:

• backlight (configure brightness icon, implement a backlight module #962 #1235)
• network_manager (configure wifi strength icon, feat: rework networkmanager module #1233)
• volume (configure volume icon)

The following locations would also expand to use this with future updates:

• battery (configure battery icon, Improve upower (battery) icon handling #935)
• music (configure volume icon)

There may also be places I've missed.

Syntaxes

This introduces three separate syntax for configuring the thresholds in an attempt to appease everybody:

Basic

Offers pre-defined low/medium/high keywords that abstract the thresholds. The thresholds are always divided into equal thirds from 0-max

icons.volume.low = "icon:volume_low"
icons.volume.medium = "icon:volume_medium"
icons.volume.high = "icon:volume_high"
---

### Dynamic

Uses an array of dynamic length to define N number of thresholds, all equally divided from `0-max`. Keys are provided in order from low-high.

```corn
icons.volume = [ "icon:volume_low" "icon_volume_medium" "icon_volume_high" ]

Manual

Uses a map to define N number of thresholds, manually specifying the lower bound for each threshold. This allows for more explicit configuration, and non-linear thresholds.

A zero threshold must be defined.

icons.volume.0 = "icon:volume_low"
icons.volume.33 = "icon:volume_medium"
icons.volume.66 = "icon:volume_high"

API

The struct would be placed inside the config:

struct Icons {
  volume: Thresholds<String>
}

Currently a single threshold_for method is provided:

let icon = icons.volume.threshold_for(curr_volume, max_volume); // Option<String>

Outstanding

• The manual format assumes integers are used as HashMap keys. This may not be supported by all formats.
• The naming is a little all over the place (level, threshold, value, ...). This needs improving.

Questions:

• Is three separate syntaxes too many? Could this introduce confusion more than it creates benefit?
  • If not, which one is the preference?
• Do we need to support negative values?
• How do we better deal with the zero value in the map syntax?
• How do we handle non-linear implementations (ie network manager)?
• This conflicts with the battery module existing thresholds option. What to do there?

---

I am looking for feedback on this! This is a bit of a "throw everything at the wall and see what sticks" approach. Anything regarding anything I've thought of above, or anything else you see.

Paging @xMAC94x @Rodrigodd as both of you have open PRs pending a decision around this - feedback from the both of you on whether you think this would work for your modules would be especially valuable.

[xMAC94x]

IMO those 3 options are probably too many. I would prefer the manual approach as it offers the most flexibility. Tbh, ironbar is configured once and it's not a huge effort for people to configure it with the manual approach.
Even more important: having a good documentation and have many examples for 1 option makes it far easier to understand, and people can just copy past a template with 3 values (0, 33, 66) to support the Basic mode.

Instead of blindly taking the first rfind key that is slower than value, we might think about allowing multiple Interpolation modes, e.g. floor and exponential. where it decides that 43 within [25, 36, 49] fits in the bucket 36. though bth, when going manual, it can all be calculated by people that really care about it and we dont need to have it in the config at all.

Its okay to have a breaking change with 0.19 when this comes. We should announce it at the release page somewhere, put it in the PR and maybe even put a warn box to the docu of the module, that the behavior changed. People will pick it up and adjust their config. thats fine for a major change version :)

There are a few more things to consider:

• we want to have thresholds for sysinfo module.
• maybe switch to BTreeMap: https://doc.rust-lang.org/std/collections/index.html#use-a-btreemap-when
• I would prob go with i32, it shouldn't matter to much.
• Allow the system to be used for css access somehow.

Edit: Maybe some way to determine the value via code would be awesome, e.g. specifying a script file or allow wasm. but that might be overengineered for now

[Rodrigodd]

In the case of the network-manager module, the default way of configuring it would be the manual method, using the thresholds that I copied from NetworkManager, replacing that strange function I made to map strength to level.

I see value in the dynamic method, in cases where a huge list of icons might be passed and the user doesn't want to tweak all threshold values whenever they want to add or remove an item. But I'm not sure if this will happen often. It may also not map well to the signal strengths NetworkManager provides, but I'm not certain, that's why I copied them directly from NetworkManager.

The basic method sounds too specialized to me, but I see the value in case you want to migrate some modules to the new system while maintaining compatibility.

Maybe there should be a script variant if handling more complex cases ever becomes necessary? This would require adding support for passing arguments to scripts, though.

[JakeStanger]

Thanks for the feedback both. Sounds like at a minimum I'll do away with the basic mode. My own thoughts below.

In the case of the network-manager module, the default way of configuring it would be the manual method

My concern here is that you would always want the non-linear logic - I'd rather keep that integrated into the code and not in the config.

The problem I see otherwise (and I guess this applies generally to the manual approach) is that if I wanted to change one of the icons for example, I now need to find which threshold value it currently uses and set that. If it's linear that is at least more predictable. Perhaps for network manager specifically we can do something similar to what you're doing and convert between the linear scale and NM's scale.

Instead of blindly taking the first rfind key that is slower than value, we might think about allowing multiple Interpolation modes, e.g. floor and exponential

This sounds like it complicates things too much to me. The current "floor" approach feels like the most predictable/understandable approach to configure.

we want to have thresholds for sysinfo module.

This sounds like a separate feature request, even if it does integrate with this. That can be achieved via external scripting already too.

Allow the system to be used for css access somehow.

Not sure what the ask is here, but again sounds like a separate request & needs fleshing out.

Maybe some way to determine the value via code

Maybe there should be a script variant if handling more complex cases ever becomes necessary?

I think that may be too complex for now, but I'm open to adding it in the future. I'm just not seeing how it would work at a technical level (oneshot script triggered on event, some new dynamic string type thing?). With the long-term goal of adding a Lua API this should work nicely.

---

I'm not too worried about low-level implementation details like the map type or performance concerns yet - I'm just prototyping currently.

[xMAC94x]

I am definitely fine with many things beeing part of a other feature request. I just want to encourage you to think at those when designing this API, so that we might be able to extend the config in the future and stay backwards compatible :)

[Rodrigodd]

My concern here is that you would always want the non-linear logic.

My concern is that I not sure if I know any better than the user what the mapping should be (although that sound like "evasion of responsibility"). For example, I just check gnome-shell, and there it uses another mapping (maybe my module should be based on this one, as it also uses GTK icons by default): https://gitlab.gnome.org/GNOME/gnome-shell/-/blob/449a4e354af82a2412cfb1ee2fe26451631aeae6/js/ui/status/network.js#L46-57

The problem I see otherwise (and I guess this applies generally to the manual approach) is that if I wanted to change one of the icons for example, I now need to find which threshold value it currently uses and set that.

I had think of putting the current default in the example configuration in the docs, so when changing the icons the user would only need to copy and paste from the example, and change them. But yes, changing the number of icons would be more annoying.