diff --git a/wmacpi/ChangeLog b/wmacpi/ChangeLog index ddc4c65..1f1e857 100644 --- a/wmacpi/ChangeLog +++ b/wmacpi/ChangeLog @@ -1,3 +1,30 @@ +2004 April 23 1.99r5 + Collected fixes for the collected fixes below . . . + + * Manpage fixes, to reflect the reality of the current code. + + * Code cleanups, to make a few things more sensible. Most notably, + the interface for setting the samplerate has changed so that it's + no longer inverted and illogical - you now say how many times you + want to sample per minute. + + * Fixed an issue with initialisation - I'd moved the power_init() + call below the options parsing code, without dealing with the -m + option properly. The end result was that if you told it to monitor + a battery number, it would fail saying the battery didn't exist. I + moved the check for this out of the options parsing and after the + power_init() call. + + * Fixed a leaking file descriptor in init_ac_adapters. + + * Implemented a way to handle changing batteries - reinitialise + the battery info periodically. I don't know of a better way to do + that, since we'd have to do all that parsing anyway to find out if + it had changed . . . + + libdockapp is waiting, but I think that's the only change left + without more bug repots . . . + 2004 April 15 1.99r4 Collected fixes for various small issues. diff --git a/wmacpi/acpi.c b/wmacpi/acpi.c index 23649fb..8b748c5 100644 --- a/wmacpi/acpi.c +++ b/wmacpi/acpi.c @@ -89,7 +89,7 @@ int main(int argc, char *argv[]) power_init(); /* we want to acquire samples over some period of time, so . . . */ for(i = 0; i < samples + 2; i++) { - for(j = 0; j < batt_count; j++) + for(j = 0; j < globals->battery_count; j++) acquire_batt_info(j); acquire_global_info(); usleep(sleep_time); @@ -98,7 +98,7 @@ int main(int argc, char *argv[]) ap = &globals->adapter; if(ap->power == AC) { printf("On AC Power"); - for(i = 0; i < batt_count; i++) { + for(i = 0; i < globals->battery_count; i++) { binfo = &batteries[i]; if(binfo->present && (binfo->charge_state == CHARGE)) { printf("; Battery %s charging", binfo->name); @@ -112,7 +112,7 @@ int main(int argc, char *argv[]) printf("\n"); } else if(ap->power == BATT) { printf("On Battery"); - for(i = 0; i < batt_count; i++) { + for(i = 0; i < globals->battery_count; i++) { binfo = &batteries[i]; if(binfo->present && (binfo->percentage >= 0)) printf(", Battery %s at %d%%", binfo->name, diff --git a/wmacpi/libacpi.c b/wmacpi/libacpi.c index bf82417..d6148f7 100644 --- a/wmacpi/libacpi.c +++ b/wmacpi/libacpi.c @@ -11,8 +11,6 @@ extern char *state[]; extern global_t *globals; -/* temp buffer */ -char buf[512]; /* local proto */ int acpi_get_design_cap(int batt); @@ -22,15 +20,15 @@ int init_batteries(void) { DIR *battdir; struct dirent *batt; - char *name, *tmp1, *tmp2; + char *name; char *names[MAXBATT]; int i, j; /* now enumerate batteries */ - batt_count = 0; + globals->battery_count = 0; battdir = opendir("/proc/acpi/battery"); if (battdir == NULL) { - perr("No batteries or ACPI not supported\n"); + pfatal("No batteries or ACPI not supported\n"); return 1; } while ((batt = readdir(battdir))) { @@ -46,23 +44,27 @@ int init_batteries(void) if (!strncmp(".", name, 1) || !strncmp("..", name, 2)) continue; - names[batt_count] = strdup(name); - batt_count++; + names[globals->battery_count] = strdup(name); + globals->battery_count++; } closedir(battdir); /* A nice quick insertion sort, ala CLR. */ - for (i = 1; i < batt_count; i++) { - tmp1 = names[i]; - j = i - 1; - while ((j >= 0) && ((strcmp(tmp1, names[j])) < 0)) { - tmp2 = names[j+1]; - names[j+1] = names[j]; - names[j] = tmp2; + { + char *tmp1, *tmp2; + + for (i = 1; i < globals->battery_count; i++) { + tmp1 = names[i]; + j = i - 1; + while ((j >= 0) && ((strcmp(tmp1, names[j])) < 0)) { + tmp2 = names[j+1]; + names[j+1] = names[j]; + names[j] = tmp2; + } } } - for (i = 0; i < batt_count; i++) { + for (i = 0; i < globals->battery_count; i++) { snprintf(batteries[i].name, MAX_NAME, "%s", names[i]); snprintf(batteries[i].info_file, MAX_NAME, "/proc/acpi/battery/%s/info", names[i]); @@ -73,13 +75,20 @@ int init_batteries(void) } /* tell user some info */ - pdebug("%d batteries detected\n", batt_count); - pinfo("libacpi: found %d batter%s\n", batt_count, - (batt_count == 1) ? "y" : "ies"); + pdebug("%d batteries detected\n", globals->battery_count); + pinfo("libacpi: found %d batter%s\n", globals->battery_count, + (globals->battery_count == 1) ? "y" : "ies"); return 0; } +/* a stub that just calls the current function */ +int reinit_batteries(void) +{ + pdebug("reinitialising batteries\n"); + return init_batteries(); +} + /* the actual name of the subdirectory under ac_adapter may * be anything, so we need to read the directory and use the * name we find there. */ @@ -104,6 +113,7 @@ int init_ac_adapters(void) continue; pdebug("found adapter %s\n", name); } + closedir(acdir); /* we /should/ only see one filename other than . and .. so * we'll just use the last value name acquires . . . */ ap->name = strdup(name); @@ -114,6 +124,13 @@ int init_ac_adapters(void) return 0; } +/* stub that does nothing but call the normal init function */ +int reinit_ac_adapters(void) +{ + pdebug("reinitialising ac adapters\n"); + return init_ac_adapters(); +} + /* see if we have ACPI support and check version */ int power_init(void) { @@ -145,6 +162,23 @@ int power_init(void) return retval; } +/* reinitialise everything, to deal with changing batteries or ac adapters */ +int power_reinit(void) +{ + FILE *acpi; + int retval; + + if (!(acpi = fopen("/proc/acpi/info", "r"))) { + pfatal("Could not reopen ACPI info file - does this system support ACPI?\n"); + return 1; + } + + if (!(retval = reinit_batteries())) + retval = reinit_ac_adapters(); + + return retval; +} + char *get_value(char *string) { char *retval; @@ -440,7 +474,7 @@ void acquire_all_batt_info(void) { int i; - for(i = 0; i < batt_count; i++) + for(i = 0; i < globals->battery_count; i++) acquire_batt_info(i); } @@ -465,7 +499,7 @@ void acquire_global_info(void) /* XXX: this needs to correctly handle the case where * any of the values used is unknown (which we flag using * -1). */ - for (i = 0; i < batt_count; i++) { + for (i = 0; i < globals->battery_count; i++) { binfo = &batteries[i]; if (binfo->present && binfo->valid) { rcap += (float)binfo->remaining_cap; diff --git a/wmacpi/libacpi.h b/wmacpi/libacpi.h index 2ba7cc8..631ef63 100644 --- a/wmacpi/libacpi.h +++ b/wmacpi/libacpi.h @@ -72,11 +72,12 @@ typedef struct { } adapter_t; typedef struct { - adapter_t adapter; int rtime; /* remaining time */ int timer; /* how long been on battery? */ int crit_level; /* anything below this is critical low */ + int battery_count; /* number of batteries found */ battery_t *binfo; /* pointer to the battery being monitored */ + adapter_t adapter; } global_t; /* @@ -120,12 +121,15 @@ typedef struct { /* Since these /are/ needed here . . . */ battery_t batteries[MAXBATT]; -int batt_count; - int verbosity; /* check if apm/acpi is enabled, etc */ int power_init(void); +/* reinitialise everything */ +int power_reinit(void); +int reinit_ac_adapters(void); +int reinit_batteries(void); + /* fill global_t with data */ void acquire_batt_info(int); void acquire_all_batt_info(void); diff --git a/wmacpi/wmacpi.1 b/wmacpi/wmacpi.1 index dd27598..381587b 100644 --- a/wmacpi/wmacpi.1 +++ b/wmacpi/wmacpi.1 @@ -92,9 +92,8 @@ Set the X display to open the window on. Set the battery to monitor initially. .TP .B \-s sample rate -Set the rate at which to sample the ACPI data (default is 100, which -translates to once every three seconds. 10 gives once every 30 seconds, -1 once every 300 seconds (five minutes), 1000 once every 0.3 seconds). +Set the rate at which to sample the ACPI data, in number of times per +minute. Minimum is 1, ie once a minute, default is 20, maximum is 600. .TP .B \-n Disable blinking power glyph when charging. Note that it still blinks when @@ -117,7 +116,7 @@ each successive use increases the verbosity. Print the version information. .TP .B \-b -Make a noise when the battery is critically low. +Enable blinking of the power glyph when charging the batteries. .TP .B \-r Disable scrolling message. diff --git a/wmacpi/wmacpi.c b/wmacpi/wmacpi.c index c9b384d..256bed3 100644 --- a/wmacpi/wmacpi.c +++ b/wmacpi/wmacpi.c @@ -64,12 +64,11 @@ typedef struct { /* globals */ Dockapp *dockapp; global_t *globals; -int count = 0; /* global timer variable */ -/* extern int verbosity; */ -/* Time for scroll updates */ -#define DEFAULT_UPDATE 150 -static int update_timeout = DEFAULT_UPDATE; +/* this gives us a variable scroll rate, depending on the importance of the + * message . . . */ +#define DEFAULT_SCROLL_RESET 150; +int scroll_reset = DEFAULT_SCROLL_RESET; /* proto for local stuff */ static void new_window(char *name); @@ -264,8 +263,6 @@ static void render_text(char *string) dockapp->tw = k; /* length of text segment */ /* re-scroll the message */ scroll_text(6, 50, 52, dockapp->tw, 1); - /* reset the scroll repeat counter */ - count = 0; } static int open_display(char *display) @@ -482,6 +479,18 @@ static void set_power_panel(void) } } +void scroll_faster(double factor) { + scroll_reset = scroll_reset * factor; +} + +void scroll_slower(double factor) { + scroll_reset = scroll_reset * factor; +} + +void reset_scroll(void) { + scroll_reset = DEFAULT_SCROLL_RESET; +} + /* * The message that needs to be displayed needs to be decided * according to a heirarchy: a message like not present needs to take @@ -517,19 +526,20 @@ static void set_message(void) if (!binfo->present) { if (state != M_NP) { state = M_NP; + reset_scroll(); render_text("not present"); } } else if (ap->power == AC) { if (binfo->charge_state == CHARGE) { if (state != M_CH) { state = M_CH; - update_timeout = DEFAULT_UPDATE; + reset_scroll(); render_text("battery charging"); } } else { if (state != M_AC) { state = M_AC; - update_timeout = DEFAULT_UPDATE; + reset_scroll(); render_text("on ac power"); } } @@ -537,25 +547,25 @@ static void set_message(void) if (binfo->state == CRIT) { if (state != M_CB) { state = M_CB; - update_timeout = 80; + scroll_faster(0.75); render_text("critical low battery"); } } else if (binfo->state == HARD_CRIT) { if (state != M_HCB) { state = M_HCB; - update_timeout = 60; + scroll_faster(0.5); render_text("hard critical low battery"); } } else if (binfo->state == LOW) { if (state != M_LB) { state = M_LB; - update_timeout = 100; + scroll_faster(0.85); render_text("low battery"); } } else { if (state != M_BATT) { state = M_BATT; - update_timeout = DEFAULT_UPDATE; + reset_scroll(); render_text("on battery"); } } @@ -604,13 +614,13 @@ void usage(char *name) { printf("%s - help\t\t[simon@dreamcraft.com.au]\n\n" "-d display\t\tdisplay on remote display \n" - "-b\t\t\tmake noise when battery is critical low (beep)\n" + "-b\t\t\tenable blinking of various UI elements\n" "-r\t\t\tdisable scrolling message\n" "-c value\t\tset critical low alarm at percent\n" "\t\t\t(default: 10 percent)\n" "-m \tbattery number to monitor\n" - "-s \trate at which to sample battery status\n" - "\t\t\tdefault 100 (once every three seconds)\n" + "-s \tnumber of times per minute to sample battery information\n" + "\t\t\tdefault 20 (once every three seconds)\n" "-n\t\t\tdo not blink\n" "-w\t\t\trun in command line mode\n" "-a \t\tsamples to average over (cli mode only)\n" @@ -638,7 +648,7 @@ void cli_wmacpi(int samples) /* we want to acquire samples over some period of time, so . . . */ for(i = 0; i < samples + 2; i++) { - for(j = 0; j < batt_count; j++) + for(j = 0; j < globals->battery_count; j++) acquire_batt_info(j); acquire_global_info(); usleep(sleep_time); @@ -647,7 +657,7 @@ void cli_wmacpi(int samples) ap = &globals->adapter; if(ap->power == AC) { printf("On AC Power"); - for(i = 0; i < batt_count; i++) { + for(i = 0; i < globals->battery_count; i++) { binfo = &batteries[i]; if(binfo->present && (binfo->charge_state == CHARGE)) { printf("; Battery %s charging", binfo->name); @@ -661,7 +671,7 @@ void cli_wmacpi(int samples) printf("\n"); } else if(ap->power == BATT) { printf("On Battery"); - for(i = 0; i < batt_count; i++) { + for(i = 0; i < globals->battery_count; i++) { binfo = &batteries[i]; if(binfo->present && (binfo->percentage >= 0)) printf(", Battery %s at %d%%", binfo->name, @@ -679,9 +689,15 @@ int main(int argc, char **argv) { char *display = NULL; int ch; - int update = 0; + int sample_count = 0; + int batt_reinit, ac_reinit; + int batt_count = 0; + int ac_count = 0; int cli = 0, samples = 1; - int samplerate = 100; + int samplerate = 20; + int sleep_rate = 10; + int sleep_time = 1000000/sleep_rate; + int scroll_count = 0; battery_t *binfo; dockapp = calloc(1, sizeof(Dockapp)); @@ -693,6 +709,15 @@ int main(int argc, char **argv) globals->crit_level = 10; battery_no = 1; + /* after this many samples, we reinit the battery and AC adapter + * information. + * XXX: make these configurable . . . */ + batt_reinit = 100; + ac_reinit = 1000; + + /* this needs to be up here because we need to know what batteries + * are available /before/ we can decide if the battery we want to + * monitor is available. */ /* parse command-line options */ while ((ch = getopt(argc, argv, "d:c:m:s:a:hnwbrvV")) != EOF) { switch (ch) { @@ -718,11 +743,6 @@ int main(int argc, char **argv) MAXBATT); return 1; } - if (battery_no > batt_count) { - fprintf(stderr, "Battery %d does not appear to be installed\n", - battery_no); - return 1; - } fprintf(stderr, "Monitoring battery %d\n", battery_no); } break; @@ -730,7 +750,7 @@ int main(int argc, char **argv) if (optarg) { samplerate = atoi(optarg); if (samplerate == 0) samplerate = 1; - if (samplerate > 3000) samplerate = 3000; + if (samplerate > 600) samplerate = 600; } else { usage(argv[0]); exit(1); @@ -764,7 +784,6 @@ int main(int argc, char **argv) dockapp->blink = 1; break; case 'r': - printf("disabling scroll\n"); dockapp->scroll = 0; break; default: @@ -774,11 +793,15 @@ int main(int argc, char **argv) } - /* see if whatever we want to use is supported */ if (power_init()) /* power_init functions handle printing error messages */ exit(1); + if (battery_no > globals->battery_count) { + pfatal("Battery %d not available for monitoring\n", battery_no); + exit(1); + } + /* check for cli mode */ if (cli) { cli_wmacpi(samples); @@ -828,7 +851,7 @@ int main(int argc, char **argv) case ButtonRelease: /* cycle through the known batteries. */ battery_no++; - battery_no = battery_no % batt_count; + battery_no = battery_no % globals->battery_count; globals->binfo = &batteries[battery_no]; binfo = globals->binfo; pinfo("changing to monitor battery %d\n", battery_no + 1); @@ -856,17 +879,39 @@ int main(int argc, char **argv) * * So, given the base rate of once every three seconds, we want to * change this test to . . . */ - if (update++ == (3000/samplerate)) { + /* Okay, this needs /fixing/ - it's ridiculous. We should be giving + * the user the option of saying how many times per minute the + * battery should be sampled, defaulting to 20 times. + * + * We sleep for one tenth of a second at a time, so 60 seconds + * translates to 600 sleeps. So, we change the default sample + * rate to 20, and the calculation below becomes . . .*/ + if (sample_count++ == ((sleep_rate*60)/samplerate)) { acquire_all_info(); - update = 0; + + /* we need to be able to reinitialise batteries and adapters, because + * they change - you can hotplug batteries on most laptops these days + * and who knows what kind of shit will be happening soon . . . */ + if (batt_count++ >= batt_reinit) { + if(reinit_batteries()) + pfatal("Oh my god, the batteries are gone!\n"); + batt_count = 0; + } + + if (ac_count++ >= ac_reinit) { + if(reinit_ac_adapters()) + pfatal("What happened to our AC adapters?!?\n"); + ac_count = 0; + } + sample_count = 0; } - if (count++ == update_timeout) { + if (scroll_count++ >= scroll_reset) { scroll_text(6, 50, 52, dockapp->tw, 1); - count = 0; + scroll_count = 0; } - /* the old code had some kind of weird crap with timers and the like. + /* The old code had some kind of weird crap with timers and the like. * As far as I can tell, it's meaningless - the time we want to display * is the time calculated from the remaining capacity, as per the * ACPI spec. The only thing I'd change is the handling of a charging @@ -886,7 +931,8 @@ int main(int argc, char **argv) /* redraw_window, if anything changed - determined inside * redraw_window. */ redraw_window(); - usleep(100000); + + usleep(sleep_time); } return 0; }