From 9464fdde2d780206e1eddba8cafbfbd4bbff83e4 Mon Sep 17 00:00:00 2001 From: Bertrik Sikken Date: Thu, 14 Aug 2008 22:35:00 +0000 Subject: Apply FS#9155 (Simplified battery bench). This is a simplification/rework of the current battery bench code. Battery measurements are now done simply once a minute (no more dependency on HDD specific timeouts) and are flushed to disk by using the ata_idle callback instead of polling ata_disk_is_active (this call is removed from the plugin API now) to make the plugin as unobtrusive as possible. This battery bench plugin also works for flash-based targets like sansa e200. git-svn-id: svn://svn.rockbox.org/rockbox/trunk@18281 a1c6a512-1295-4272-9138-f99709370657 --- apps/plugin.c | 11 +- apps/plugin.h | 21 ++-- apps/plugins/battery_bench.c | 281 +++++++++++++++++++------------------------ 3 files changed, 138 insertions(+), 175 deletions(-) (limited to 'apps') diff --git a/apps/plugin.c b/apps/plugin.c index 24082de637..6ebb6fad61 100644 --- a/apps/plugin.c +++ b/apps/plugin.c @@ -220,6 +220,7 @@ static const struct plugin_api rockbox_api = { gui_synclist_limit_scroll, gui_synclist_do_button, gui_synclist_set_title, + gui_syncyesno_run, simplelist_info_init, simplelist_show_list, @@ -263,9 +264,12 @@ static const struct plugin_api rockbox_api = { read_line, settings_parseline, ata_sleep, - ata_disk_is_active, ata_spin, ata_spindown, +#if USING_ATA_CALLBACK + register_ata_idle_func, + unregister_ata_idle_func, +#endif /* USING_ATA_CALLBACK */ reload_directory, create_numbered_filename, file_exists, @@ -593,6 +597,7 @@ static const struct plugin_api rockbox_api = { tagcache_get_next, tagcache_retrieve, tagcache_search_finish, + tagcache_get_numeric, #endif #ifdef HAVE_ALBUMART @@ -602,11 +607,7 @@ static const struct plugin_api rockbox_api = { /* new stuff at the end, sort into place next time the API gets incompatible */ -#ifdef HAVE_TAGCACHE - tagcache_get_numeric, -#endif - gui_syncyesno_run, }; int plugin_load(const char* plugin, const void* parameter) diff --git a/apps/plugin.h b/apps/plugin.h index 2ff2381890..373795b145 100644 --- a/apps/plugin.h +++ b/apps/plugin.h @@ -88,6 +88,7 @@ void* plugin_get_buffer(size_t *buffer_size); #include "buffering.h" #include "tagcache.h" #include "viewport.h" +#include "ata_idle_notify.h" #ifdef HAVE_ALBUMART #include "albumart.h" @@ -129,12 +130,12 @@ void* plugin_get_buffer(size_t *buffer_size); #define PLUGIN_MAGIC 0x526F634B /* RocK */ /* increase this every time the api struct changes */ -#define PLUGIN_API_VERSION 120 +#define PLUGIN_API_VERSION 121 /* update this to latest version if a change to the api struct breaks backwards compatibility (and please take the opportunity to sort in any new function which are "waiting" at the end of the function table) */ -#define PLUGIN_MIN_API_VERSION 120 +#define PLUGIN_MIN_API_VERSION 121 /* plugin return codes */ enum plugin_status { @@ -324,6 +325,9 @@ struct plugin_api { bool (*gui_synclist_do_button)(struct gui_synclist * lists, unsigned *action, enum list_wrap wrap); void (*gui_synclist_set_title)(struct gui_synclist *lists, char* title, int icon); + enum yesno_res (*gui_syncyesno_run)(const struct text_message * main_message, + const struct text_message * yes_message, + const struct text_message * no_message); void (*simplelist_info_init)(struct simplelist_info *info, char* title, int count, void* data); bool (*simplelist_show_list)(struct simplelist_info *info); @@ -367,9 +371,12 @@ struct plugin_api { int (*read_line)(int fd, char* buffer, int buffer_size); bool (*settings_parseline)(char* line, char** name, char** value); void (*ata_sleep)(void); - bool (*ata_disk_is_active)(void); void (*ata_spin)(void); void (*ata_spindown)(int seconds); +#if USING_ATA_CALLBACK + void (*register_ata_idle_func)(ata_idle_notify function); + void (*unregister_ata_idle_func)(ata_idle_notify function, bool run); +#endif /* USING_ATA_CALLBACK */ void (*reload_directory)(void); char *(*create_numbered_filename)(char *buffer, const char *path, const char *prefix, const char *suffix, @@ -746,6 +753,7 @@ struct plugin_api { bool (*tagcache_retrieve)(struct tagcache_search *tcs, int idxid, int tag, char *buf, long size); void (*tagcache_search_finish)(struct tagcache_search *tcs); + long (*tagcache_get_numeric)(const struct tagcache_search *tcs, int tag); #endif #ifdef HAVE_ALBUMART @@ -756,13 +764,6 @@ struct plugin_api { /* new stuff at the end, sort into place next time the API gets incompatible */ -#ifdef HAVE_TAGCACHE - long (*tagcache_get_numeric)(const struct tagcache_search *tcs, int tag); -#endif - - enum yesno_res (*gui_syncyesno_run)(const struct text_message * main_message, - const struct text_message * yes_message, - const struct text_message * no_message); }; diff --git a/apps/plugins/battery_bench.c b/apps/plugins/battery_bench.c index 0cc028ebd0..90e9b405c8 100644 --- a/apps/plugins/battery_bench.c +++ b/apps/plugins/battery_bench.c @@ -26,7 +26,6 @@ PLUGIN_HEADER #define BATTERY_LOG "/battery_bench.txt" #define BUF_SIZE 16000 -#define DISK_SPINDOWN_TIMEOUT 3600 #define EV_EXIT 1337 @@ -198,12 +197,15 @@ struct batt_info unsigned short flags; } bat[BUF_SIZE/sizeof(struct batt_info)]; -struct thread_entry *thread_id; -struct event_queue thread_q; +#define BUF_ELEMENTS (sizeof(bat)/sizeof(struct batt_info)) + +static struct thread_entry *thread_id; +static struct event_queue thread_q; +static bool in_usb_mode; +static unsigned int buf_idx; bool exit_tsr(bool reenter) { - bool exit = true; (void)reenter; rb->lcd_clear_display(); rb->lcd_puts_scroll(0, 0, "Batt.Bench is currently running."); @@ -213,9 +215,7 @@ bool exit_tsr(bool reenter) #endif rb->lcd_update(); - if (rb->button_get(true) != BATTERY_OFF) - exit = false; - if (exit) + if (rb->button_get(true) == BATTERY_OFF) { rb->queue_post(&thread_q, EV_EXIT, 0); rb->thread_wait(thread_id); @@ -236,174 +236,119 @@ bool exit_tsr(bool reenter) unsigned long thread_stack[THREAD_STACK_SIZE/sizeof(long)]; #if CONFIG_CHARGING || defined(HAVE_USB_POWER) -unsigned int charge_state(void) +static unsigned int charge_state(void) { unsigned int ret = 0; #if CONFIG_CHARGING - if(rb->charger_inserted()) + if (rb->charger_inserted()) ret = BIT_CHARGER; #if CONFIG_CHARGING == CHARGING_MONITOR - if(rb->charging_state()) + if (rb->charging_state()) ret |= BIT_CHARGING; #endif #endif #ifdef HAVE_USB_POWER - if(rb->usb_powered()) + if (rb->usb_powered()) ret |= BIT_USB_POWER; #endif return ret; } #endif -void thread(void) + +static bool flush_buffer(void) { - bool got_info = false, timeflag = false, in_usb_mode = false; - int fd, buffelements, tick = 1, i = 0, skipped = 0, exit = 0; - int fst = 0, lst = 0; /* first and last skipped tick */ - unsigned int last_voltage = 0; -#if CONFIG_CHARGING || defined(HAVE_USB_POWER) - unsigned int last_state = 0; -#endif - long sleep_time = 5 * HZ; - - struct queue_event ev; + int fd, secs; + unsigned int i; - buffelements = sizeof(bat)/sizeof(struct batt_info); + /* don't access the disk when in usb mode, or when no data is available */ + if (in_usb_mode || (buf_idx == 0)) + { + return false; + } -#ifndef HAVE_FLASH_STORAGE - if(rb->global_settings->disk_spindown > 1) - sleep_time = (rb->global_settings->disk_spindown - 1) * HZ; -#endif + fd = rb->open(BATTERY_LOG, O_RDWR | O_CREAT | O_APPEND); + if (fd < 0) + { + return false; + } - do + for (i = 0; i < buf_idx; i++) { - if(!in_usb_mode && got_info && - (exit || timeflag || rb->ata_disk_is_active()) ) - { - int last, secs, j, temp = skipped; - - fd = rb->open(BATTERY_LOG, O_RDWR | O_CREAT | O_APPEND); - if(fd < 0) - exit = 1; - else - { - do - { - if(skipped) - { - last = buffelements; - fst /= HZ; - lst /= HZ; - rb->fdprintf(fd,"-Skipped %d measurements from " - "%02d:%02d:%02d to %02d:%02d:%02d-\n",skipped, - HMS(fst),HMS(lst)); - skipped = 0; - } - else - { - last = i; - i = 0; - } - - for(j = i; j < last; j++) - { - secs = bat[j].ticks/HZ; - rb->fdprintf(fd, - "%02d:%02d:%02d, %05d, %03d%%, " - "%02d:%02d, %04d, %04d" + secs = bat[i].ticks/HZ; + rb->fdprintf(fd, + "%02d:%02d:%02d, %05d, %03d%%, " + "%02d:%02d, %04d, " #if CONFIG_CHARGING - ", %c" + " %c" #if CONFIG_CHARGING == CHARGING_MONITOR - ", %c" + ", %c" #endif #endif #ifdef HAVE_USB_POWER - ", %c" + ", %c" #endif - "\n", - - HMS(secs), secs, bat[j].level, - bat[j].eta / 60, bat[j].eta % 60, - bat[j].voltage, - temp + 1 + (j-i) + "\n", + + HMS(secs), secs, bat[i].level, + bat[i].eta / 60, bat[i].eta % 60, + bat[i].voltage #if CONFIG_CHARGING - ,(bat[j].flags & BIT_CHARGER)?'A':'-' + , (bat[i].flags & BIT_CHARGER) ? 'A' : '-' #if CONFIG_CHARGING == CHARGING_MONITOR - ,(bat[j].flags & BIT_CHARGING)?'C':'-' + , (bat[i].flags & BIT_CHARGING) ? 'C' : '-' #endif #endif #ifdef HAVE_USB_POWER - ,(bat[j].flags & BIT_USB_POWER)?'U':'-' + , (bat[i].flags & BIT_USB_POWER) ? 'U' : '-' #endif - - ); - if(!j % 100 && !j) /* yield() at every 100 writes */ - rb->yield(); - } - temp += j - i; - - }while(i != 0); - - rb->close(fd); - tick = *rb->current_tick; - got_info = false; - timeflag = false; - } - } - else - { - unsigned int current_voltage; - if( -#if CONFIG_CODEC == SWCODEC - !rb->pcm_is_playing() -#else - !rb->mp3_is_playing() -#endif - && (*rb->current_tick - tick) > DISK_SPINDOWN_TIMEOUT * HZ) - timeflag = true; - - if(last_voltage != (current_voltage=rb->battery_voltage()) + ); + } + rb->close(fd); + + buf_idx = 0; + return true; +} + + +void thread(void) +{ + bool exit = false; + char *exit_reason = "unknown"; + long sleep_time = 60 * HZ; + struct queue_event ev; + int fd; + + in_usb_mode = false; + buf_idx = 0; + + while (!exit) + { + /* add data to buffer */ + if (buf_idx < BUF_ELEMENTS) + { + bat[buf_idx].ticks = *rb->current_tick; + bat[buf_idx].level = rb->battery_level(); + bat[buf_idx].eta = rb->battery_time(); + bat[buf_idx].voltage = rb->battery_voltage(); #if CONFIG_CHARGING || defined(HAVE_USB_POWER) - || last_state != charge_state() + bat[buf_idx].flags = charge_state(); #endif - ) - { - if(i == buffelements) - { - if(!skipped++) - fst = bat[0].ticks; - i = 0; - } - else if(skipped) - { - skipped++; - lst = bat[i].ticks; - } - bat[i].ticks = *rb->current_tick; - bat[i].level = rb->battery_level(); - bat[i].eta = rb->battery_time(); - last_voltage = bat[i].voltage = current_voltage; -#if CONFIG_CHARGING || defined(HAVE_USB_POWER) - bat[i].flags = last_state = charge_state(); -#endif - i++; - got_info = true; - } - - } + buf_idx++; + rb->register_ata_idle_func(flush_buffer); + } - if(exit) - { - if(exit == 2) - rb->splash(HZ, -#ifdef HAVE_LCD_BITMAP - "Exiting battery_bench..."); -#else - "bench exit"); -#endif - return; + /* What to do when the measurement buffer is full: + 1) save our measurements to disk but waste some power doing so? + 2) throw away measurements to save some power? + The choice made here is to save the measurements. It is quite unusual + for this to occur because it requires > 16 hours of no disk activity. + */ + if (buf_idx == BUF_ELEMENTS) { + flush_buffer(); } + /* sleep some time until next measurement */ rb->queue_wait_w_tmo(&thread_q, &ev, sleep_time); switch (ev.id) { @@ -416,20 +361,38 @@ void thread(void) rb->usb_acknowledge(SYS_USB_DISCONNECTED_ACK); break; case SYS_POWEROFF: - exit = 1; + exit_reason = "power off"; + exit = true; break; case EV_EXIT: - exit = 2; +#ifdef HAVE_LCD_BITMAP + rb->splash(HZ, "Exiting battery_bench..."); +#else + rb->splash(HZ, "bench exit"); +#endif + exit_reason = "plugin exit"; + exit = true; break; } - } while (1); + } + + /* unregister flush callback and flush to disk */ + rb->unregister_ata_idle_func(flush_buffer, true); + + /* log end of bench and exit reason */ + fd = rb->open(BATTERY_LOG, O_RDWR | O_CREAT | O_APPEND); + if (fd >= 0) + { + rb->fdprintf(fd, "--Battery bench ended, reason: %s--\n", exit_reason); + rb->close(fd); + } } #ifdef HAVE_LCD_BITMAP typedef void (*plcdfunc)(int x, int y, const unsigned char *str); -void put_centered_str(const char* str, plcdfunc putsxy, int lcd_width, int line) +static void put_centered_str(const char* str, plcdfunc putsxy, int lcd_width, int line) { int strwdt, strhgt; rb->lcd_getstringsize(str, &strwdt, &strhgt); @@ -452,7 +415,7 @@ int main(void) rb->lcd_clear_display(); rb->lcd_setfont(FONT_SYSFIXED); - for(i = 0; i<(int)(sizeof(msgs)/sizeof(char *)); i++) + for (i = 0; i<(int)(sizeof(msgs)/sizeof(char *)); i++) put_centered_str(msgs[i],rb->lcd_putsxy,LCD_WIDTH,i+1); #else rb->lcd_puts_scroll(0, 0, "Batt.Bench."); @@ -473,7 +436,7 @@ int main(void) do { button = rb->button_get(true); - switch(button) + switch (button) { case BATTERY_ON: #ifdef BATTERY_RC_ON @@ -484,23 +447,24 @@ int main(void) case BATTERY_OFF: #ifdef BATTERY_RC_OFF case BATTERY_RC_OFF: -#endif +#endif return PLUGIN_OK; - - default: if(rb->default_event_handler(button) == SYS_USB_CONNECTED) + + default: + if (rb->default_event_handler(button) == SYS_USB_CONNECTED) return PLUGIN_USB_CONNECTED; } }while(!on); fd = rb->open(BATTERY_LOG, O_RDONLY); - if(fd < 0) + if (fd < 0) { fd = rb->open(BATTERY_LOG, O_RDWR | O_CREAT); - if(fd >= 0) + if (fd >= 0) { rb->fdprintf(fd, "This plugin will log your battery performance in a\n" - "file (%s) every time the disk is accessed (or every hour).\n" + "file (%s) every minute.\n" "To properly test your battery:\n" "1) Select and playback an album. " "(Be sure to be more than the player's buffer)\n" @@ -512,13 +476,10 @@ int main(void) "Do not enter another plugin during the test or else the " "logging activity will end.\n\n" "P.S: You can decide how you will make your tests.\n" - "Just don't open another plugin to be sure that your log " - "will continue.\n" - "M/DA (Measurements per Disk Activity) shows how many times\n" - "data was logged in the buffer between Disk Activity.\n\n" + "Just don't open another plugin to be sure that your log " + "will continue.\n\n" "Battery type: %d mAh Buffer Entries: %d\n" - " Time:, Seconds:, Level:, Time Left:, Voltage[mV]:," - " M/DA:" + " Time:, Seconds:, Level:, Time Left:, Voltage[mV]:" #if CONFIG_CHARGING ", C:" #endif @@ -530,7 +491,7 @@ int main(void) #endif "\n" ,BATTERY_LOG,rb->global_settings->battery_capacity, - BUF_SIZE / (unsigned)sizeof(struct batt_info)); + (int)BUF_ELEMENTS); rb->close(fd); } else @@ -548,10 +509,10 @@ int main(void) } rb->queue_init(&thread_q, true); /* put the thread's queue in the bcast list */ - if((thread_id = rb->create_thread(thread, thread_stack, + if ((thread_id = rb->create_thread(thread, thread_stack, sizeof(thread_stack), 0, "Battery Benchmark" IF_PRIO(, PRIORITY_BACKGROUND) - IF_COP(, CPU))) == NULL) + IF_COP(, CPU))) == NULL) { rb->splash(HZ, "Cannot create thread!"); return PLUGIN_ERROR; @@ -562,4 +523,4 @@ int main(void) return PLUGIN_OK; } -#endif +#endif /* SIMULATOR */ -- cgit v1.2.3