tobi-wan-kenobi / bumblebee-status

bumblebee-status is a modular, theme-able status line generator for the i3 window manager.
https://bumblebee-status.readthedocs.io/en/main/
MIT License
1.22k stars 228 forks source link

More robust identification of changes to public IPs #916

Closed tfwiii closed 2 years ago

tfwiii commented 2 years ago

Added secondary check for potential changes in public IP and a small bug fix arising from reliance on util.location

Added a second check for indications of possible change of public IP address since netifaces does not properly handle all Linux routing tables (ref: http://linux-ip.net/html/routing-tables.html) which can lead to changes being missed if only looking at defauilt route. Consequently an additional check for changes to the network interface names/numbers was added to further help in identifying potentially notable changes. Note that netifaces is no longer being maintained so the underlying issue with its handling of gateways is unlikely to be resolved (https://github.com/al45tair/netifaces). An alternative would be to use direct calls to the OS like 'ip route list table all' (note the difference to 'ip route list all') but this might lead to unpredicatble results between distributions/flavours so probably best to stick with a library for now. all') but this might lead to unpredicatble results between distributions/flavours so probably best to stick with a library for now.

Introduced time.sleep(2) following calls to util.location.reset() since it can take util.location a while to update following a call to .reset() which can cause calls to .location_info() to return unpredicatable/unuseful results.

tobi-wan-kenobi commented 2 years ago

Thank you very much!

The changes look good to me, I have 1 question and 1 remark, though:

  1. I do not understand the need for time.sleep(2) after location.reset(). Looking at the location code, this change should take effect immediately (it just clears a variable), what was the issue you experienced?
  2. There is a leftover print() in an exception handler - Can you please replace that with a logging.error()? That way, it ends up in the log and does not break the i3bar protocol.

Thank you kindly for your contributions, much valued!

tfwiii commented 2 years ago

Thank you very much!

The changes look good to me, I have 1 question and 1 remark, though:

1. I do not understand the need for `time.sleep(2)` after `location.reset()`. Looking at the location code, this change should take effect immediately (it just clears a variable), what was the issue you experienced?

2. There is a leftover `print()` in an exception handler - Can you please replace that with a logging.error()? That way, it ends up in the log and does not break the i3bar protocol.

Thank you kindly for your contributions, much valued!

Hi,

I found that calls to location.reset(), when followed quickly by a call to location.location_info() would sometimes result in null values being returned. I'm guess as a result of something of a race condition while the new values were being remotely fetched. A pause seemed to help address this. A more ideal solution might be to to implement a mechanism in the location module which ensured that calls to location.location_info() are only serviced once a successful reset() had been completed...

Happy for the print to be changed or removed as suits you best. Sorry, I thought I had removed all my debugging but missed that one!

On another subject - I have moved to Wayland (Sway) recently and don't plan on ever going back to X! Would you be interested in some Wayland friendly modules being written?

tobi-wan-kenobi commented 2 years ago

Ah, I see. I will need to look at making the location calls synchronous then (I thought they were, but does not look like it)

I am not using Wayland myself, but I would be happy to add any wayland-specific modules you come up with!

(full disclosure, I actually moved to AwesomeWM as my main driver, but I am committed to keep bumblebee-status up-to-date, useable and useful, since so many people invested effort in this small project)

tobi-wan-kenobi commented 2 years ago

@tfwiii I have merged the PR, adapted the logging & changed the reset a bit (to remove the sleep).

If this re-introduces your errors, please let me know & I will fix them.

Thanks for your contribution!