Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

State Check on Startup #8

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

luguenth
Copy link

Hey there! 👋 Thanks for creating this awesome extension - it's been really helpful for my workflow!

I wanted to contribute a small but useful feature that I think others might benefit from too:

feat: Add state checking functionality for toggle commands

I added the ability to check the current state of services/commands before toggling. The extension now uses command exit codes to determine toggle states:

  • Exit code 0: Toggle shows as enabled
  • Non-zero exit code: Toggle shows as disabled

For example, I'm using it with Docker like this:
systemctl is-active docker.service

  • Returns exit code 0 when Docker is running
  • Returns exit code 3 when it's stopped
  • The toggle automatically reflects the actual Docker state 🎉

This is super helpful if you (like me) frequently start/stop services and want the UI to always show the correct state.

Let me know if you'd like me to adjust anything or if you have questions! Happy to help make this even better. 😊

@StorageB
Copy link
Owner

I'm glad to hear you this extension has been useful for you!

I like the idea of this, but have a few questions/issues after testing this out a bit:

  1. This was working for the first button, but not any additional buttons. I think the error is here (button 2 for example) this._indicator2.toggle1.checked = result; It should be toggle2 for button2. However, I'm not sure if this line is even needed. At the end of the extension startup, it calls refreshIndicator.call(this); which recreates the buttons from the const MyIndicator1 = GObject.registerClass class (example for toggle 1) and sets the state with this line: this.toggle1.checked = toggleState1;
  2. When using the "check state dynamically" option, if the exit status is 0 for the entered command, the Toggle Off command runs at startup even if the "run command at startup" option is turned off. When turned on, it was running both the Toggle On and Off commands.
  3. It looks like this will check the command immediately at startup. When I added the run command at startup option, I had to give the option of delaying when the command runs after boot to allow other services/processes to load, otherwise the command might run before the associated process had finished loading. I'm wondering if that might be an issue with this as well?
  4. In the schemas file, checkregex3-setting is listed but not used. Also, the summaries and descriptions for checkcommand3-setting buttons 3 and up all reference button 2.

Let me know your thoughts!

@psihozefir
Copy link

psihozefir commented Jan 11, 2025

Hi! I am trying to use this button with my Philips TV. There is an utility called philipstv that I am using to turn the TV on and off using a key binding - win+t. I want to be able to use the mouse, so this extension became interresting. philipstv can question the TV for its power status, because only the panel is off, but not the android system when the TV is in standby (actually, the TV is in standby, but it can be awakened using wake-on-lan). The problem is the TV can be in any state when the computer starts and its state can be toggled by something else (e.g. a remote). So I need the button to be able to query the state of the TV and reflect its status on startup. philipstv power get returns on or off and I think the output should also be an option when reading the status, not only the exit code. The output also needs a conversion from random boolean representation to boolean, because some commands return yes/no, others on/off, others yet 0/1 or 1/0, etc. so this should be configurable.

@Jules-Bertholet
Copy link

I think the output should also be an option when reading the status, not only the exit code.

You can wrap the phillipstv command in a script that does this check. I don’t think it makes sense to handle the complexity of this in the extension directly.

@psihozefir
Copy link

psihozefir commented Jan 11, 2025

I think the output should also be an option when reading the status, not only the exit code.

You can wrap the phillipstv command in a script that does this check. I don’t think it makes sense to handle the complexity of this in the extension directly.

I understand... I know I can do philipstv power get | grep -q 'on'. However, how can you make the button reflect the state of the controlled device? Many commands return true for both states, just like philipstv, because it is best practice. You return a false exit code on error only and the return status of a check is not an error.

@StorageB
Copy link
Owner

StorageB commented Jan 13, 2025

I agree with @Jules-Bertholet - good idea.

@psihozefir
In your script you can set what the exit code will be when running the script. So in your script you would have the command to determine the state of the TV. If true, send exit code 0. If false send exit code 1. Then (when this feature is implemented in this extension) the extension should be able to set the state of the button on startup by reading the exit code from your script.

The way @luguenth has this set up, you just enter the command you what to run to check the exit code. For you, you would just enter the command to run your script.

Here is the example script I was using to test this:

#!/bin/bash

 # set VARIABLE to true or false for testing
VARIABLE="true"

# Check the variable and set the exit code
if [ "$VARIABLE" = "true" ]; then
    echo "Variable is true"
    exit 0
else
    echo "Variable is false"
    exit 1
fi

@luguenth
Copy link
Author

To address your questions @StorageB :

I was admittedly a bit sloppy with the code. Initially, I wanted to implement a regex check on stdout, but I soon realized I couldn't retrieve the stdout from the async process call. I ended up using the exit code since it was the only information I could get about whether my command did anything. This might also answer @psihozefir's question about why I'm only looking at the exit code. If anyone can find a way to properly check the command output, that would be really helpful, as we could then set the state based on a regex pattern.

Regarding the Toggle On/Off on startup issue (even without the option being active) - I'll look into it, but I'm short on time right now, so it might take me 2-3 weeks. And yes, I agree that delaying the state check would be much better. I'll also work on cleaning up the code for the multiple buttons.

@StorageB
Copy link
Owner

StorageB commented Jan 31, 2025

@luguenth After thinking about this a bit more, I think your original idea of looking at the command output and doing a regex check on an input word/phrase the user specifies would be the best option instead of looking at the exit code. Then it could work for use cases like what @psihozefir was trying to set up.

I've had some luck getting command output using GLib.spawn_async_with_pipes instead of the GLib.spawn_asynccommand I was using, but still working on making it usable.

I can continue working with the code you provided to see if I can get this to work - unless you've already made some progress, and in that case I could share what I have so far.

Edit - I think I finally have a working concept. I'll post a new version here for testing once it's fully working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants