firecat53 / networkmanager-dmenu

Control NetworkManager via dmenu
MIT License
811 stars 74 forks source link

Delay at startup for checking if bluetooth is enabled when the bluetooth systemd service is not running - Proposed solutions #131

Closed itshog closed 9 months ago

itshog commented 9 months ago

Since the addition of bluetoothctl as a way to control bluetooth connections (9b3f5aa, 9a48cdf), a two seconds delay at startup was added https://github.com/firecat53/networkmanager-dmenu/blob/0230e2da8b722f6a6bb8471b3364aca3dd22e1af/networkmanager_dmenu#L168 for checking if bluetooth is enabled. In theory the command in question should execute much faster, but in practice having the bluetooth systemd service disabled or stopped causes bluetoothctl to hang, consuming the whole two seconds as stated in the code itself:

        # Times out in 2 seconds, otherwise bluetoothctl will hang if bluetooth
        # service isn't running.

In my opinion this behaviour is kind of frustrating, especially for laptop users who want some energy-saving: since I rarely use bluetooth, I keep the adapter off and the systemd service disabled most of the time, and I manually start it only when needed. It's a bit annoying that this choice causes a two seconds delay in an application that, before the introduction of the bluetooth feature, was extremely snappy: for me, one of the main reason for using dmenu-like programs for controlling my system has always been speed...

I know that I could circumvent the problem by uninstalling the package that provides the bluetoothctl binary, but I'd like to keep it installed for the rare occasions in which I have to use bluetooth.

I propose the following solutions, in order of preference:

  1. Make the whole bluetooth part optional by having something like control_bluetooth = True|False in the configuration file, so users who don't need this feature can opt out. (This is my preferred solution, but I admit I'm kind of biased as I almost never use bluetooth)
  2. Add only the option of disabling bluetoothctl (use_bluetoothctl = True|False) (IMO having an option for bluetooth as a whole makes much more sense, but this may work as well)
  3. Perform other checks before calling bluetoothctl show, like making sure that the bluetooth systemd service is running (This may solve the problem, but I don't know how well it translates to systems without systemd. Also, maybe there are other checks needed for having a "clean" experience)
  4. Make only the timeout configurable, so users can at least reduce it to something less intrusive
  5. Find a completely different way for checking if bluetooth is enabled
  6. Leave the code as is, but specify in the README that having bluez-utils installed while bluetooth.service is not running may lead to this delay (It wouldn't actually solve the problem, but at least the user would be informed)

Let me know if one of these proposed solutions could work, and thanks for the amazing project!

itshog commented 9 months ago

Edit: after looking at Void and Artix package sources for bluez I found out that the bluetooth system service always starts bluetoothd, no matter if using systemd, runit or openrc, so a viable approach for implementing the third solution could be simply calling pidof bluetoothd before executing bluetoothctl. Anyway, I tried to implement solution 2 (https://github.com/itshog/networkmanager-dmenu/commit/ea13f44b274cb39d97d98fd9953940c24e532521) and 3 (https://github.com/itshog/networkmanager-dmenu/commit/7501f2dd48a63ab018b8113076bd40f61fcab0e9) myself, and they seem to effectively solve the problem. Should I make a pull request?

firecat53 commented 9 months ago

That would be great, thanks!

itshog commented 9 months ago

That would be great, thanks!

Which one of the proposed solutions and implementations do you prefer? In my opinion, the best solution would be to modularize the program, making the whole bluetooth part optional, but I believe this would require some kind of refactor of the whole script, especially if we start modularizing other things.

In the meantime, I guess 3 makes the most sense, since it doesn't require a configuration option and simply adds an extra check before calling bluetoothctl (also, this doesn't exclude further modifications to the bluetooth part: if sometimes in the future the aforementioned modularization happens, this check would still be necessary to ensure we don't have the delay when bluetoothd is not running).

The only concern that I have regards the possibility of successfully running bluetoothctl without bluetoothd in the background: if this is possibile, adding the extra check would prevent a legitimate use case and might be detrimental to some users. From this point of view, 2 might be a "safest" solution, even if personally I don't like it that much.

What do you think?

veltza commented 9 months ago

bluetoothctl does not work without bluetoothd because they belong to the same Bluetooth stack. So option 3 should be sufficient.

firecat53 commented 9 months ago

I agree, option 3 seems to be the easiest and cleanest.

itshog commented 9 months ago

I agree, option 3 seems to be the easiest and cleanest.

Thanks, I opened a pull request