brndnmtthws / conky

Light-weight system monitor for X, Wayland (sort of), and other things, too
https://conky.cc
GNU General Public License v3.0
7.27k stars 620 forks source link

execbar broken #580

Closed origintopleft closed 6 years ago

origintopleft commented 6 years ago
${execbar "echo 'scale=2; 51/100' | bc -q"}
${execbar "echo .51"}
${execbar "echo 51"}

all return empty bars, with and without the extra quoting around the whole command. cpubar works fine.

conky 1.10.8, Gentoo Linux.

conky 1.10.8 compiled Thu Jul 12 21:56:59 PDT 2018 for Linux 4.17.4-gentoo-ishimura x86_64

Compiled in features:

System config file: /etc/conky/conky.conf
Package library path: /usr/lib64/conky

 General:
  * portmon
  * IPv6
  * iconv
  * wireless
  * builtin default configuration
  * Imlib2
  * iostats
  * ncurses
  * Internationalization support
  * PulseAudio

 Lua bindings:
  * Cairo
  * Imlib2
 X11:
  * Xdamage extension
  * Xinerama extension (virtual display)
  * Xshape extension (click through)
  * XDBE (double buffer extension)
  * Xft
  * ARGB visual
  * Own window

 Default values:
  * Netdevice: eth0
  * Local configfile: $HOME/.conkyrc
  * Localedir: /usr/share/locale
  * Maximum netdevices: 64
  * Maximum text size: 16384
  * Size text buffer: 256
lasers commented 6 years ago

Now I think about it... {battery_bar} is probably broken too. Thanks.

su8 commented 6 years ago

@lasers ${execbar echo 51} without " and ' works fine.

edit:

execbar

lasers commented 6 years ago
execbar (height),(width) command
    Same as exec, except if the first value returned is a value
    between 0-100, it will use  that  number to  draw  a  horizontal
    bar.  The  height and width parameters are optional, and default
    to the default_bar_height and default_bar_width config settings,
    respectively.

@su8 According to the docs, it makes sense to use quotes here.

Maybe the issue is wrong or outdated docs? We need to fix major https://github.com/brndnmtthws/conky/issues/197 so we can start testing each variable one at a time and update the docs accordingly.

How does execbar currently work? I guess nobody updated the docs during the rewritten.

EDIT: What about ${battery_bar} ?

lasers commented 6 years ago

Also, maybe @lavacano201014's percent is too low. It's not even 1%. This works for me...

conky.config = {out_to_x=false,out_to_console=true}
conky.text = [[
    ${execbar echo 'scale=2; 100/3' | bc -q}
    ${execbar echo 50.51}
    ${execbar echo 50}
    ---------------
]]
su8 commented 6 years ago

the following works:

${execbar echo "51"}
${execbar echo '51'}
lasers commented 6 years ago

Yeah, I guess we don't want to wrap command.

1) Is it possible to support that too? 2) Should we support that? 3) We may have to compare this with other variables first before making a decision here. 4) Document this. Document this. Document this.

lasers commented 6 years ago

@lavacano201014 Let us know if this works okay for you too so we can close this. Thanks.

su8 commented 6 years ago
diff --git a/src/exec.cc b/src/exec.cc
index 40c97c91..7e0ec6af 100644
--- a/src/exec.cc
+++ b/src/exec.cc
@@ -55,6 +55,28 @@ struct execi_data {
 static FILE *pid_popen(const char *command, const char *mode, pid_t *child) {
   int ends[2];
   int parentend, childend;
+  char cmd[256];
+  char *str1 = cmd;
+  const char *str2 = command;
+  int detected_quote = 0;
+  int skip = 0;
+
+  for (; *str2; str2++) {
+    if (0 == skip) {
+      if (*str2 == '"' || *str2 == '\'') {
+        detected_quote = 1;
+        skip = 1;
+        continue;
+      }
+    }
+    if ('\0' == *(str2+1)) {
+      if (*str2 == '"' || *str2 == '\'') {
+        continue;
+      }
+    }
+    *str1++ = *str2;
+  }
+

   // by running pipe after the strcmp's we make sure that we don't have to
   // create a pipe and close the ends if mode is something illegal
@@ -92,7 +114,7 @@ static FILE *pid_popen(const char *command, const char *mode, pid_t *child) {
     if (fcntl(childend, F_DUPFD, 0) == -1) { perror("fcntl()"); }
     close(childend);

-    execl("/bin/sh", "sh", "-c", command, (char *)nullptr);
+    execl("/bin/sh", "sh", "-c", cmd, (char *)nullptr);
     _exit(EXIT_FAILURE);  // child should die here, (normally execl will take
                           // care of this but it can fail)
   }
lasers commented 6 years ago
    --------------- 1
    __________
    #####_____
    __________
    --------------- 2
    __________
    __________
    __________
    --------------- 3
    #####_____
    __________
    #####_____
    --------------- 4
    #####_____
    __________
    #####_____

Excellent. Good for me! Can you push a branch to this repo or make new branch and push it?

origintopleft commented 6 years ago

Now it almost works.

The bar keeps blinking back to empty, like I'm feeding it zeroes, but I triple checked and made sure, and the only time ${execbar} is actually receiving zeroes is when the progress is actually 0%.

I'm trying to record a GIF of it but x11grab was removed from recent versions of ffmpeg and I don't know how to get xcbgrab to work yet.

origintopleft commented 6 years ago

Context: I have a Python script that gets MPRIS information and displays it (I don't see an ${mpris_title} and friends in Conky docs, but that's another issue for another day). This is the part that manages the execbar:

if "mpris:length" in meta:
    pos = math.floor(ply.player.Position / 1000000)
    length = math.floor(meta["mpris:length"] / 1000000)
    barperc = math.floor((pos / length) * 100)

    print("${{color2}}{0} ${{color1}}${{execbar echo {2}}} ${{color2}}{1}".format(pos, length, barperc))

That last print statement outputs this to conky (figure I'm 140 seconds into a 238 second song):

${color2}140 ${color1}${execbar echo 58} ${color2}238

Switched to using latest git commit.

origintopleft commented 6 years ago

myfile

Here's the recording of what it's doing.

lasers commented 6 years ago

@lavacano201014 Try https://github.com/brndnmtthws/conky/pull/591 branch. @su8 added some overflow checks.

origintopleft commented 6 years ago

still doing it, but it's probably good to get me on the right branch anyway, thanks :V

lasers commented 6 years ago

@lavacano201014 I pulled in unknown modifications to #591 from @su8. It's also unknown if he addresses this on-off issue. Can you try to see if it's still happening?

su8 commented 6 years ago

I'm not getting any flickering - https://youtu.be/OwY5qRyzMAQ

${execbar shuf -i 1-100 -n 1}
lasers commented 6 years ago

@lavacano201014 Is it the only bar you're trying to print? Can we have your config too?

lasers commented 6 years ago

@su8

conky.config = {
    out_to_x=true,
    out_to_console=false,
    own_window=true,
}
conky.text = [[
    1 bad   ${execbar 30,500 shuf -i 0-100 -n 1}

    2 bad   ${execbar 30,500 shuf -i 0-100 -n 1}

    3 bad   ${execbar 30,500 shuf -i 0-100 -n 1}

    4 bad   ${execbar 30,500 shuf -i 0-100 -n 1}

    ${color darkorange}${hr}${color}

    5 good  ${execbar 30,500 shuf -i 1-100 -n 1}

    6 good  ${execbar 30,500 shuf -i 0-99 -n 1}

    7 good  ${execbar 30,500 shuf -i 0-98 -n 1}

    8 good  ${execbar 30,500 shuf -i 0-97 -n 1}

    9 bad   ${execbar 30,500 shuf -i 0-100 -n 1}
]]
su8 commented 6 years ago

own_window=true seems to flicker on my system from time to time.

origintopleft commented 6 years ago

I'm on my way to work at the moment so I can't provide my full config, but:

origintopleft commented 6 years ago

Here's my config and the two scripts it references, sorry I'm late

lasers commented 6 years ago

My bar looks okay. I'll try different songs. EDIT: Okay, I saw it. :-) Minor tweaks to make it work with other machines better.

diff --git a/temps.py b/temps2.py
index aeea6ee..df1dfd4 100755
--- a/temps.py
+++ b/temps2.py
@@ -9,7 +9,7 @@ sensorstr = sensorproc.stdout.decode()
 sensordata = json.loads(sensorstr)

 def get_temp(sensor):
-    return sensordata[sensor]["temp1"]["temp1_input"]
+    return sensordata.get(sensor, {}).get('temp1', {}).get("temp1_input")

 def pretty_print_temp(sensor, name):
     print("${{color0}}{0}:${{alignr}}${{color1}}{1}⁰C    ".format(name, get_temp(sensor)))
diff --git a/mpris.py b/mpris2.py
index 41db558..c40b6d3 100755
--- a/mpris.py
+++ b/mpris2.py
@@ -2,8 +2,6 @@

 import math
 import sys
-import subprocess
-
 import dbus
 from dbus.mainloop.glib import DBusGMainLoop
 import pympris
@@ -12,9 +10,11 @@ busloop = DBusGMainLoop()
 bus = dbus.SessionBus(mainloop=busloop)

 plys = list(pympris.available_players())
-ply = pympris.MediaPlayer(plys[0], bus)
-
-if ply.player.PlaybackStatus != "Playing":
+if plys:
+    ply = pympris.MediaPlayer(plys[0], bus)
+    if ply.player.PlaybackStatus != "Playing":
+        sys.exit(0)
+else:
     sys.exit(0)

 meta = ply.player.Metadata
@@ -48,4 +48,4 @@ elif protocol == "spotify":
 else:
     print("   from unrecognized protocol " + protocol)

-print("") # formatting
+print("")  # formatting
lasers commented 6 years ago

lavacano201014 Now it almost works. The bar keeps blinking back to empty, like I'm feeding it zeroes, but I triple checked and made sure, and the only time ${execbar} is actually receiving zeroes is when the progress is actually 0%.

I can confirm this. @su8 The bar flickers everytime we get a new percent, eg 28 to 29. The script never gave 0 to ${execbar echo %s}.

pos:2413, length:7547, barperc:31
pos:2414, length:7547, barperc:31
pos:2415, length:7547, barperc:31  <-- bar displays 31 percent
pos:2416, length:7547, barperc:32  <-- bar displays 0 percent aka flickering
pos:2417, length:7547, barperc:32  <-- bar displays 32 percent
pos:2418, length:7547, barperc:32
pos:2419, length:7547, barperc:32

EDIT: Everytime a percent changed, I get this in the log.

conky: reading exec value failed (perhaps it's not the correct format?)
su8 commented 6 years ago

${execbar echo %s} is causing the following

conky: reading exec value failed (perhaps it's not the correct format?)
conky: X Error: Display 56383457adc0

because it expects a number in the form of double to be scanned/returned

https://github.com/brndnmtthws/conky/blob/bbaa0e9ac8d8024245c2dde676ed4b6376f8300e/src/exec.cc#L198

lasers commented 6 years ago

Why does this not flicker?

conky.config = {
    out_to_x=true,
    out_to_console=false,
    own_window=true,
    update_interval=0.1,
}
conky.text = [[${execbar 30,500 shuf -i 0-100 -n 1}]]
lasers commented 6 years ago

@su8 I think this might have something to do with it...

execp command
       Executes  a shell command and displays the output in conky. Warning: this takes a lot more resources
       than other variables. I'd recommend coding wanted behaviour in C/C++ and posting a patch. This  dif‐
       fers  from  $exec in that it parses the output of the command, so you can insert things like ${color
       red}hi!${color} in your script and have it correctly parsed by  Conky.  Caveats:  Conky  parses  and
       evaluates the output of $execp every time Conky loops, and then destroys all the objects. If you try
       to use anything like $execi within an $execp statement, it will functionally run at the same  inter‐
       val that the $execp statement runs, as it is created and destroyed at every interval.
su8 commented 6 years ago

It doesn't execute execi/execp, but exec_cb which is calling pid_popen.

lasers commented 6 years ago

His script mpris.py is using execbar for the bar... eg ${execp .... ${execbar} }

su8 commented 6 years ago

Yes the behaviour for execbar is the same as described above. All exec/execpi/execp/execbar functions call exec_cb which on their side use pid_popen.

lasers commented 6 years ago

Just to be clear...

because it expects a number in the form of double to be scanned/returned

When the flicker happens, it happened because of the percent change. You said it's complaining about lack of double, but it always stopped complaining and started working after the first one... It's on-off with every percent change.

su8 commented 6 years ago

What happens when you apply this patch, does it flicker again :question:

diff --git a/src/exec.cc b/src/exec.cc
index bc6bf47b..a320430f 100644
--- a/src/exec.cc
+++ b/src/exec.cc
@@ -199,7 +199,7 @@ static inline double get_barnum(const char *buf) {
     NORM_ERR(
         "reading exec value failed (perhaps it's not the "
         "correct format?)");
-    return 0.0;
+    return 10.0;
   }
   if (barnum > 100.0 || barnum < 0.0) {
     NORM_ERR(

edit: use ${execbar echo "%s"}

lasers commented 6 years ago

To clarify, there is no real flickering. The bar is responding to "not a correct format" 0.0 percent. With this patch, the bar is flickering with 10.0 percent instead.

su8 commented 6 years ago

That's what I wanted to know, thanks.

lasers commented 6 years ago

@su8 Now you know. What now?

su8 commented 6 years ago

With this config I get flickering:

conky.config = {
    out_to_x=true,
    out_to_console=false,
    own_window=true,
    update_interval=0.1,
}
conky.text = [[
${execbar shuf -i 1-90 -n 1}
${execbar shuf -i 1-80 -n 1}
${execbar shuf -i 1-70 -n 1}
${execbar shuf -i 1-60 -n 1}
${execbar shuf -i 1-50 -n 1}
${execbar shuf -i 1-40 -n 1}
${execbar shuf -i 1-30 -n 1}
${execbar shuf -i 1-20 -n 1}
]]

When I set update_interval=1 it takes a second or two for the bars to expand to their full width and cause 1 flickering, otherwise I don't get any flickers with update_interval=1 when ran in the long run. Too much drawing/painting is happening perhaps :question:

lasers commented 6 years ago

@su8 but you don't get conky: reading exec value failed (perhaps it's not the correct format?) in the console here like when you run his conky config...

When I tried this...

conky.config = {
    out_to_x=true,
    out_to_console=false,
    own_window=true,
    update_interval=1,
    default_bar_width=500,
}
conky.text = [[
${execbar shuf -i 10-90 -n 1}
${execbar shuf -i 10-80 -n 1}
${execbar shuf -i 10-70 -n 1}
${execbar shuf -i 10-60 -n 1}
${execbar shuf -i 10-50 -n 1}
${execbar shuf -i 10-40 -n 1}
${execbar shuf -i 10-30 -n 1}
${execbar shuf -i 10-20 -n 1}
]]

Sure, there is real flickering, but none of that 0.0 invalid format messages. They all stay above 10.

su8 commented 6 years ago

I think we solved this issue as "echo 51" and "echo 'scale=2; 51/100' | bc -q" no longer return empty bars, the flickering alone requires another ticket to be opened.

lasers commented 6 years ago
conky.config = {
    out_to_x=true,
    out_to_console=false,
    own_window=true,
    update_interval=1,
    default_bar_width=500,
}
conky.text = [[
    BAR: ${execbar "shuf -i 10-20 -n 1"}
    ${execp /tmp/echo0.py}
    ${execp /tmp/echo1.py}
    ${execp /tmp/echo2.py}
    ${execp /tmp/echo3.py}
    ${execp /tmp/echo4.py}
]]
echo0.py:#!/usr/bin/env python
echo0.py:print("BAR: ${execbar 'echo 33'}")  # static
echo1.py:#!/usr/bin/env python
echo1.py:from random import randint
echo1.py:print("BAR: ${execbar 'echo %s'}" % randint(10, 100))
echo2.py:#!/usr/bin/env python
echo2.py:from random import randint
echo2.py:print("BAR: ${execbar 'echo %s'}" % randint(10, 100))
echo3.py:#!/usr/bin/env python
echo3.py:from random import randint
echo3.py:print("BAR: ${execbar 'echo %s'}" % randint(10, 100))
echo4.py:#!/usr/bin/env python
echo4.py:from random import randint
echo4.py:print("BAR: ${execbar 'echo %s'}" % randint(10, 100))

You'll see lot of reading exec value failed (perhaps it's not the correct format?).

EDIT: I made a mistake. 10, 100 instead of 1, 99. Why do we keep getting 0 here? EDIT: Why do we keep getting conky: reading exec value failed (perhaps it's not the correct format?)?

:question: :question: :question: :question: :question: :question: :question: :question: :question:

su8 commented 6 years ago

pid_popen doesn't do variable resolution. Try this

#!/usr/bin/env python
print("BAR: ${darkorange}${hr}")
lasers commented 6 years ago

${color darkorange}

su8 commented 6 years ago

Drop the ' single quotes and it works:

#!/usr/bin/env python
from random import randint
print("BAR: ${execbar echo %s}" % randint(1, 99))
lasers commented 6 years ago

@su8 If you try your 10.0 patch, you'll see they're always at 10 or above. :-) Will try quotes.

su8 commented 6 years ago

Leave it working for like 30 seconds and you'll see that the bars are moving.

lasers commented 6 years ago

The bars should never exceed below 10 due to randint(10, 100).

su8 commented 6 years ago

I've set the following:

diff --git a/src/exec.cc b/src/exec.cc
index bc6bf47b..af00a0df 100644
--- a/src/exec.cc
+++ b/src/exec.cc
@@ -195,6 +195,7 @@ static void remove_deleted_chars(char *string) {
 static inline double get_barnum(const char *buf) {
   double barnum;

+  printf("%s\n", buf);
   if (sscanf(buf, "%lf", &barnum) != 1) {
     NORM_ERR(
         "reading exec value failed (perhaps it's not the "

and in the console we get empty string returned back to us, except sometimes we get numbers back to us:


conky: reading exec value failed (perhaps it's not the correct format?)

conky: reading exec value failed (perhaps it's not the correct format?)

conky: reading exec value failed (perhaps it's not the correct format?)

conky: reading exec value failed (perhaps it's not the correct format?)
37 # received

conky: reading exec value failed (perhaps it's not the correct format?)
35 # received
lasers commented 6 years ago

Yes. You understand the issue now. That's the real issue he had been trying to understand.

su8 commented 6 years ago

So the code (exec_cb) evaluates the script ${execp /tmp/echo1.py} and returns nothing, then it sees the ${execbar echo 44} variable and parses it again, and evaluates that to number.

lasers commented 6 years ago

Thoughts on skipping empty strings as a solution? EDIT: Oh I get it. The execp thing?

su8 commented 6 years ago

Try this patch and see the numbers being printed correctly. It opens another pid_popen process and somewhere the the information gets lost.

diff --git a/src/exec.cc b/src/exec.cc
index bc6bf47b..dafd4d8c 100644
--- a/src/exec.cc
+++ b/src/exec.cc
@@ -147,6 +147,7 @@ void exec_cb::work() {
   std::shared_ptr<FILE> fp;
   char b[0x1000];

+  /* printf("%s\n",std::get<0>(tuple).c_str()); */
   if (FILE *t = pid_popen(std::get<0>(tuple).c_str(), "r", &childpid)) {
     fp.reset(t, fclose);
   } else {
@@ -195,6 +196,7 @@ static void remove_deleted_chars(char *string) {
 static inline double get_barnum(const char *buf) {
   double barnum;

+  /* printf("%s\n", buf); */
   if (sscanf(buf, "%lf", &barnum) != 1) {
     NORM_ERR(
         "reading exec value failed (perhaps it's not the "
@@ -222,11 +224,8 @@ static inline double get_barnum(const char *buf) {
  */
 void fill_p(const char *buffer, struct text_object *obj, char *p,
             unsigned int p_max_size) {
-  if (obj->parse) {
-    evaluate(buffer, p, p_max_size);
-  } else {
     snprintf(p, p_max_size, "%s", buffer);
-  }
+  

   remove_deleted_chars(p);
 }
lasers commented 6 years ago

10-4. I see this. Good numbers everywhere.