greshake / i3status-rust

Very resourcefriendly and feature-rich replacement for i3status, written in pure Rust
GNU General Public License v3.0
2.85k stars 470 forks source link

Add connectivity report (cell network) on kdeconnect block #1873

Closed Atreyagaurav closed 1 year ago

Atreyagaurav commented 1 year ago

Closes https://github.com/greshake/i3status-rust/issues/1872

Atreyagaurav commented 1 year ago

With this: format = " $icon {$bat_icon $bat_charge|}{ $notif_icon|} {$network_icon $network|}"image

I wasn't able to test if it does show red when there is no network. Simply disabling the SIM gave me "unknown" network. So might need more investigation on that. Maybe the refreshed isn't being captured properly or something.

bim9262 commented 1 year ago

@Atreyagaurav, I'm going to mark this as draft while you work out how to handle the disabled sim. Also check out the fmt and clippy checks as they aren't passing.

Atreyagaurav commented 1 year ago

I wasn't able to check the fmt as cargo fmt was ok, and rust analyzer was giving me errors about async fn. But I checked the contributing.md, and ran rustfmt 2021 should be ok now.

For the SIM disable stuff looks like https://invent.kde.org/network/kdeconnect-kde/-/tree/master/plugins/connectivity-report says the signal strength is signalStrength (int) [0..=4]: The signal strength but it seems to be -1 when the SIM is disabled. It is probably emitted from something else (strength value changed signal) as I didn't get any signal when I disabled the sim when I did:

dbus-monitor "type='signal',sender='org.kde.kdeconnect',interface='org.kde.kdeconnect.device.connectivity_report'"

Now it works when I disable the sim. And should work also when the strength is zero.

Atreyagaurav commented 1 year ago

It said it can't apply the patch, I did copy and paste the most things. And it refused to compile as the notif_count was now Option<usize>, so I changed the function for it. Should be ok.

MaxVerevkin commented 1 year ago

From https://invent.kde.org/network/kdeconnect-kde/-/tree/master/plugins/connectivity-report

signalStrengths (object<string, object>): Maps each SIM (subscription ID) to the following object: networkType (string): One of "5G", "LTE", "CDMA", "EDGE", "GPRS", "GSM", "HSPA", "UMTS", "CDMA2000", "iDEN", "Unknown" signalStrength (int) [0..=4]: The signal strength

Does the dbus interface allow us to get the information about all sims?

fn cellular_network_strength(&self) -> zbus::Result<i32>;

Does this return the strength of the first sim? Or the average?

bim9262 commented 1 year ago

Does the dbus interface allow us to get the information about all sims?

Looks like it's just the first sim: https://invent.kde.org/network/kdeconnect-kde/-/blob/master/plugins/connectivity-report/connectivity_reportplugin.cpp#L45

Atreyagaurav commented 1 year ago

Maybe we can submit a pull request to the plugin [For future]. Doesn't make sense to just discard the information if kdeconnect app sends the signal for all. But it might complicate things now with backward compatibility. IDK.

Atreyagaurav commented 1 year ago

I made the "$network" give network percentage and "$network_type" give the type of network. Having the percentage might be desirable when there's no progression. I think that's most of what I'd want in this block. We can improve it for a few things but we might have to change upstream, either the plugin in kdeconnect app or desktop for those.

bim9262 commented 1 year ago

I think that $network might be confusing for network strength (both in name and value) perhaps $network_strength would be better. Also I don't think that representing number of bars as a percent is really ideal. Maybe doing bars (up to 4) would be a good option.

MaxVerevkin commented 1 year ago

Also I don't think that representing number of bars as a percent is really ideal

Why though?

Atreyagaurav commented 1 year ago

Also I don't think that representing number of bars as a percent is really ideal. Maybe doing bars (up to 4) would be a good option.

If the icon theme supports it, then we'll have the progression on the symbol itself. But without that I think the percent is easiest to understand compared to other numbers (as people might not know 4 is maximum here, even that seems debatable as I've been using it for a while, and I've never seen it give 100% even though the document says it's 0..=4).

I did think about using something that's free and available here: https://github.com/greshake/i3status-rust/issues/1886 But I think that might be a good idea to implement globally in a different pull request for all other percentage related values than for just this one. And it's also easier if we use bar, as we'd know the maximum value is 100.

bim9262 commented 1 year ago

Also I don't think that representing number of bars as a percent is really ideal

Why though?

"There’s no industry standard that ties cell phone signal strength to the number of bars on your phone. It's up to each carrier or phone manufacturer to decide what's 1, 2, 3, 4, or full bars on their service. So, 2 bars on AT&T could be 3 bars on T-Mobile, or 4 bars on Verizon." https://www.wilsonamplifiers.com/blog/how-to-read-cell-phone-signal-strength-the-right-way/

For example on a phone with 5 bars you might see:

Signal Strength General Results
-50 to -79 dBm Considered great signal (4 to 5 bars)
-80 to -89 dBm Considered good signal (3 to 4 bars)
-90 to -99 dBm Considered average signal (2 to 3 bars)
-100 to -109 dBm Considered poor signal (1 to 2 bars)
-110 to -120 dBm Considered very poor signal (0 to 1 bar)

In order to calculate something as a percentage you need to be able to say what the $max$ is in a linear scale:

$$\frac{max-current}{max}$$

but for a logarithmic scale like dBm, we need something like:

$$ \frac{\log(current) - log(min)}{log(max)-log(min)} $$

On my phone I observed that -117 dBm is 1 bar while -118 dBm is 0 bars, but the change of 1 dBm doesn't represent a 25% drop in network strength. If we say the $max=-50, min=-120$ then -118 dBm is 1.92% strength while -117 dBm is 2.89% strength, however if we say that $max=-44, min=-140$ (this seem to be the full spectrum of signal strength) then -118 dBm is 14.77% strength while -117 dBm is 15.51% strength.

To me it seems misleading to say the strength is 0%, 25%, 50%, 75%, or 100%

Atreyagaurav commented 1 year ago

Ohk, I guess since we don't have this information sent to us, and it only sends the bar information, we can't really fix it from our side. We might have to ask for changes in the kdeconnect app and the plugin to send us the exact information about connectivity.

So for our fix, do you want to just remove the percentage thing for now and be satisfied as the icon is progression? Or do you want me to add the vertical bar thing (▁ ▂ ▃ ▄ ▅ ▆ ▇: as a progression) instead of percentage as format variable?

printf "%b " "\u2581" "\u2582" "\u2583" "\u2584" "\u2585" "\u2586" "\u2587\n"

These are not limited by icon themes so we can add them to all of them.

EDIT: we can use empty (×) for 0 and then only 4 of these bars for 1..=4.

bim9262 commented 1 year ago

So for our fix, do you want to just remove the percentage thing for now and be satisfied as the icon is progression? Or do you want me to add the vertical bar thing (▁ ▂ ▃ ▄ ▅ ▆ ▇: as a progression) instead of percentage as format variable?

I'll defer to @MaxVerevkin since I was the original descent, if the reason I gave was reason enough to not include the network strength.

MaxVerevkin commented 1 year ago

So for our fix, do you want to just remove the percentage thing for now and be satisfied as the icon is progression? Or do you want me to add the vertical bar thing (▁ ▂ ▃ ▄ ▅ ▆ ▇: as a progression) instead of percentage as format variable?

I'll defer to @MaxVerevkin since I was the original descent, if the reason I gave was reason enough to not include the network strength.

Personally I don't mind if we expose network strength as percentage (we already do so for wifi strength, with somewhat arbitrary conversion from dBm to percents). As for the vertical bar thing, I think it is best to implement it as a formatter option (#1886) (this could be useful for wifi and cpu too). Note that it doesn't really matter if we have strength as a percentage or a number of bars, both could be formatted as "vertical bars" (but the latter would require an additional argument to set the "max value" to 4).

To me it seems misleading to say the strength is 0%, 25%, 50%, 75%, or 100%

Having more precision would be better, sure.

Maybe doing bars (up to 4) would be a good option.

Do you mean a number from 0 to 4? Or graphical representation?

Atreyagaurav commented 1 year ago

Yeah, people could choose to not use the percentage. And we can possibly add in the documentation that it's not the correct representation, but it can later be used for other representation. And we're working on what information we have, the bars not being accurate representation is mostly the upstream problem.

It's trivial to remove that, so if you guys think we should remove it, just let me know. At least for my usecase I only want to know whether the network is very bad or not. So for me it works if it shows me 0% or 25%.

This seems to be where the signal strength is converted to number of bars for KDE connect Android app:

https://invent.kde.org/network/kdeconnect-android/-/blob/master/src/org/kde/kdeconnect/Plugins/ConnectivityReportPlugin/ConnectivityReportPlugin.java#L145

So I think even if the networks use different ways, as long as the same library is used and the strength given by the device is absolute value, it's consistent? That could also be the reason even when my phone shows 3 or 4 bars, as the actual value isn't changed much the kdeconnect block is stuck at 75% for me. I'm not knowledgeable in this, just my 2 cents.

bim9262 commented 1 year ago

Do you mean a number from 0 to 4? Or graphical representation?

Sorry, I meant a graphical representation.

MaxVerevkin commented 1 year ago

Thanks @Atreyagaurav for addressing all the feedback and thanks @bim9262 for reviewing!