| Message ID | 20201109205713.380179-1-sbabic@denx.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Fix handle of total downloaded bytes in progress | expand |
Hi Stefano, > commit 5738732 disables the number of downloaded bytes because buggy, > this fix the behavior and ensures that the value is correctly forwareded > to the listeners. > > Signed-off-by: Stefano Babic <sbabic@denx.de> > --- > core/notifier.c | 7 ++++--- > core/progress_thread.c | 11 ++++++++--- > include/progress.h | 9 --------- > include/progress_ipc.h | 1 + > tools/swupdate-progress.c | 4 ++-- > 5 files changed, 15 insertions(+), 17 deletions(-) > > diff --git a/core/notifier.c b/core/notifier.c > index 276e155..e4ca3c6 100644 > --- a/core/notifier.c > +++ b/core/notifier.c > @@ -339,15 +339,16 @@ static void process_notifier (RECOVERY_STATUS status, int event, int level, cons > */ > static void progress_notifier (RECOVERY_STATUS status, int event, int level, const char *msg) > { > + int dwl_percent = 0; > + unsigned long long dwl_bytes = 0; > (void)level; > > /* Check just in case a process want to send an info outside */ > if (status != PROGRESS) > return; > > - if (event == RECOVERY_DWL) { > - struct progress_dwl_data *pdwl = (struct progress_dwl_data *)msg; > - swupdate_download_update(pdwl->dwl_percent, pdwl->dwl_bytes); > + if (event == RECOVERY_DWL && (sscanf(msg, "%d-%llu", &dwl_percent, &dwl_bytes) == 2)) { > + swupdate_download_update(dwl_percent, dwl_bytes); > return; > } > > diff --git a/core/progress_thread.c b/core/progress_thread.c > index cc556c1..7c424fc 100644 > --- a/core/progress_thread.c > +++ b/core/progress_thread.c > @@ -97,7 +97,7 @@ static void _swupdate_download_update(unsigned int perc, unsigned long long tota > pthread_mutex_lock(&pprog->lock); > if (perc != pprog->msg.dwl_percent) { > pprog->msg.dwl_percent = perc; > - totalbytes = totalbytes; > + pprog->msg.dwl_bytes = totalbytes; > send_progress_msg(); > } > pthread_mutex_unlock(&pprog->lock); > @@ -138,8 +138,13 @@ void swupdate_download_update(unsigned int perc, unsigned long long totalbytes) > * Not called by main process, for example by suricatta or Webserver > */ > if (pid == getpid()) { > - struct progress_dwl_data *pdwl = (struct progress_dwl_data *)info; > - pdwl->dwl_percent = perc; > + /* > + * Notify can just receive a string as message > + * so it is necessary to encode further information as string > + * and decode them in the notifier, in this case > + * the progress_notifier > + */ > + snprintf(info, sizeof(info) - 1, "%d-%llu", perc, totalbytes); > notify(PROGRESS, RECOVERY_DWL, TRACELEVEL, info); > return; > } > diff --git a/include/progress.h b/include/progress.h > index 738e59f..9297cf9 100644 > --- a/include/progress.h > +++ b/include/progress.h > @@ -11,15 +11,6 @@ > #include <swupdate_status.h> > #include <progress_ipc.h> > > -/* > - * Internal structures to be used to forward progress data > - */ > - > -struct progress_dwl_data { > - unsigned int dwl_percent; /* % downloaded data */ > - unsigned long long dwl_bytes; /* total of bytes to be downloaded */ > -}; > - > /* > * Internal SWUpdate functions to drive the progress > * interface. Common progress definitions for internal > diff --git a/include/progress_ipc.h b/include/progress_ipc.h > index f508eda..2efa61d 100644 > --- a/include/progress_ipc.h > +++ b/include/progress_ipc.h > @@ -27,6 +27,7 @@ struct progress_msg { > unsigned int magic; /* Magic Number */ > RECOVERY_STATUS status; /* Update Status (Running, Failure) */ > unsigned int dwl_percent; /* % downloaded data */ > + unsigned long long dwl_bytes; /* total of bytes to be downloaded */ > unsigned int nsteps; /* No. total of steps */ > unsigned int cur_step; /* Current step index */ > unsigned int cur_percent; /* % in current step */ > diff --git a/tools/swupdate-progress.c b/tools/swupdate-progress.c > index ecc0b40..754d56f 100644 > --- a/tools/swupdate-progress.c > +++ b/tools/swupdate-progress.c > @@ -327,11 +327,11 @@ int main(int argc, char **argv) > fill_progress_bar(bar, sizeof(bar), msg.cur_percent); > > if (!silent) { > - fprintf(stdout, "[ %.*s ] %d of %d %d%% (%s) dwl %d%%\r", > + fprintf(stdout, "[ %.*s ] %d of %d %d%% (%s), dwl %d%% of %llu bytes\r", > bar_len, > bar, > msg.cur_step, msg.nsteps, msg.cur_percent, > - msg.cur_image, msg.dwl_percent); > + msg.cur_image, msg.dwl_percent, msg.dwl_bytes); > fflush(stdout); > } > > -- > 2.25.1 > Just found that this breaks core/syslog.c by it emitting messages like FATAL_UNKNOWN 15-217716224 with the message being the <percent>-<filesize> encoding. The 'FATAL_' prefix stems from this line in core/syslog.c syslog(logprio, "%s%s %s\n", ((error != (int)RECOVERY_NO_ERROR) ? "FATAL_" : ""), statusMsg, msg); where error == RECOVERY_DWL should also not result in the 'FATAL_' prefix. Then, switch(status) { should also cover case PROGRESS: statusMsg = "PROGRESS"; break; and probably also SUBPROCESS for completeness. This silences the FATAL alarm but still you get logged the encoded information in <percent>-<filesize> syntax. IMO, it should print something more friendly to human interpretation :) Kind regards, Christian
Hi Christian, On 18.11.20 17:06, Christian Storm wrote: > Hi Stefano, > >> commit 5738732 disables the number of downloaded bytes because buggy, >> this fix the behavior and ensures that the value is correctly forwareded >> to the listeners. >> >> Signed-off-by: Stefano Babic <sbabic@denx.de> >> --- >> core/notifier.c | 7 ++++--- >> core/progress_thread.c | 11 ++++++++--- >> include/progress.h | 9 --------- >> include/progress_ipc.h | 1 + >> tools/swupdate-progress.c | 4 ++-- >> 5 files changed, 15 insertions(+), 17 deletions(-) >> >> diff --git a/core/notifier.c b/core/notifier.c >> index 276e155..e4ca3c6 100644 >> --- a/core/notifier.c >> +++ b/core/notifier.c >> @@ -339,15 +339,16 @@ static void process_notifier (RECOVERY_STATUS status, int event, int level, cons >> */ >> static void progress_notifier (RECOVERY_STATUS status, int event, int level, const char *msg) >> { >> + int dwl_percent = 0; >> + unsigned long long dwl_bytes = 0; >> (void)level; >> >> /* Check just in case a process want to send an info outside */ >> if (status != PROGRESS) >> return; >> >> - if (event == RECOVERY_DWL) { >> - struct progress_dwl_data *pdwl = (struct progress_dwl_data *)msg; >> - swupdate_download_update(pdwl->dwl_percent, pdwl->dwl_bytes); >> + if (event == RECOVERY_DWL && (sscanf(msg, "%d-%llu", &dwl_percent, &dwl_bytes) == 2)) { >> + swupdate_download_update(dwl_percent, dwl_bytes); >> return; >> } >> >> diff --git a/core/progress_thread.c b/core/progress_thread.c >> index cc556c1..7c424fc 100644 >> --- a/core/progress_thread.c >> +++ b/core/progress_thread.c >> @@ -97,7 +97,7 @@ static void _swupdate_download_update(unsigned int perc, unsigned long long tota >> pthread_mutex_lock(&pprog->lock); >> if (perc != pprog->msg.dwl_percent) { >> pprog->msg.dwl_percent = perc; >> - totalbytes = totalbytes; >> + pprog->msg.dwl_bytes = totalbytes; >> send_progress_msg(); >> } >> pthread_mutex_unlock(&pprog->lock); >> @@ -138,8 +138,13 @@ void swupdate_download_update(unsigned int perc, unsigned long long totalbytes) >> * Not called by main process, for example by suricatta or Webserver >> */ >> if (pid == getpid()) { >> - struct progress_dwl_data *pdwl = (struct progress_dwl_data *)info; >> - pdwl->dwl_percent = perc; >> + /* >> + * Notify can just receive a string as message >> + * so it is necessary to encode further information as string >> + * and decode them in the notifier, in this case >> + * the progress_notifier >> + */ >> + snprintf(info, sizeof(info) - 1, "%d-%llu", perc, totalbytes); >> notify(PROGRESS, RECOVERY_DWL, TRACELEVEL, info); >> return; >> } >> diff --git a/include/progress.h b/include/progress.h >> index 738e59f..9297cf9 100644 >> --- a/include/progress.h >> +++ b/include/progress.h >> @@ -11,15 +11,6 @@ >> #include <swupdate_status.h> >> #include <progress_ipc.h> >> >> -/* >> - * Internal structures to be used to forward progress data >> - */ >> - >> -struct progress_dwl_data { >> - unsigned int dwl_percent; /* % downloaded data */ >> - unsigned long long dwl_bytes; /* total of bytes to be downloaded */ >> -}; >> - >> /* >> * Internal SWUpdate functions to drive the progress >> * interface. Common progress definitions for internal >> diff --git a/include/progress_ipc.h b/include/progress_ipc.h >> index f508eda..2efa61d 100644 >> --- a/include/progress_ipc.h >> +++ b/include/progress_ipc.h >> @@ -27,6 +27,7 @@ struct progress_msg { >> unsigned int magic; /* Magic Number */ >> RECOVERY_STATUS status; /* Update Status (Running, Failure) */ >> unsigned int dwl_percent; /* % downloaded data */ >> + unsigned long long dwl_bytes; /* total of bytes to be downloaded */ >> unsigned int nsteps; /* No. total of steps */ >> unsigned int cur_step; /* Current step index */ >> unsigned int cur_percent; /* % in current step */ >> diff --git a/tools/swupdate-progress.c b/tools/swupdate-progress.c >> index ecc0b40..754d56f 100644 >> --- a/tools/swupdate-progress.c >> +++ b/tools/swupdate-progress.c >> @@ -327,11 +327,11 @@ int main(int argc, char **argv) >> fill_progress_bar(bar, sizeof(bar), msg.cur_percent); >> >> if (!silent) { >> - fprintf(stdout, "[ %.*s ] %d of %d %d%% (%s) dwl %d%%\r", >> + fprintf(stdout, "[ %.*s ] %d of %d %d%% (%s), dwl %d%% of %llu bytes\r", >> bar_len, >> bar, >> msg.cur_step, msg.nsteps, msg.cur_percent, >> - msg.cur_image, msg.dwl_percent); >> + msg.cur_image, msg.dwl_percent, msg.dwl_bytes); >> fflush(stdout); >> } >> >> -- >> 2.25.1 >> > > Just found that this breaks core/syslog.c by it emitting messages like > FATAL_UNKNOWN 15-217716224 > with the message being the <percent>-<filesize> encoding. > > The 'FATAL_' prefix stems from this line in core/syslog.c > syslog(logprio, "%s%s %s\n", ((error != (int)RECOVERY_NO_ERROR) ? "FATAL_" : ""), statusMsg, msg); > where error == RECOVERY_DWL should also not result in the 'FATAL_' prefix. > > Then, > switch(status) { > should also cover > case PROGRESS: statusMsg = "PROGRESS"; break; > and probably also SUBPROCESS for completeness. > > This silences the FATAL alarm but still you get logged the encoded > information in <percent>-<filesize> syntax. IMO, it should print > something more friendly to human interpretation :) Well, but this generates an overflow of dummy messages for syslog without meaning, as the message is addressed to the progress interface instead of the logging. I will tend to not log unknown sources instead ofadding them. So like: case DONE: statusMsg = "DONE"; break; default: return; Best regards, Stefano
Hi Stefano, > >> commit 5738732 disables the number of downloaded bytes because buggy, > >> this fix the behavior and ensures that the value is correctly forwareded > >> to the listeners. > >> > >> Signed-off-by: Stefano Babic <sbabic@denx.de> > >> --- > >> core/notifier.c | 7 ++++--- > >> core/progress_thread.c | 11 ++++++++--- > >> include/progress.h | 9 --------- > >> include/progress_ipc.h | 1 + > >> tools/swupdate-progress.c | 4 ++-- > >> 5 files changed, 15 insertions(+), 17 deletions(-) > >> > >> diff --git a/core/notifier.c b/core/notifier.c > >> index 276e155..e4ca3c6 100644 > >> --- a/core/notifier.c > >> +++ b/core/notifier.c > >> @@ -339,15 +339,16 @@ static void process_notifier (RECOVERY_STATUS status, int event, int level, cons > >> */ > >> static void progress_notifier (RECOVERY_STATUS status, int event, int level, const char *msg) > >> { > >> + int dwl_percent = 0; > >> + unsigned long long dwl_bytes = 0; > >> (void)level; > >> > >> /* Check just in case a process want to send an info outside */ > >> if (status != PROGRESS) > >> return; > >> > >> - if (event == RECOVERY_DWL) { > >> - struct progress_dwl_data *pdwl = (struct progress_dwl_data *)msg; > >> - swupdate_download_update(pdwl->dwl_percent, pdwl->dwl_bytes); > >> + if (event == RECOVERY_DWL && (sscanf(msg, "%d-%llu", &dwl_percent, &dwl_bytes) == 2)) { > >> + swupdate_download_update(dwl_percent, dwl_bytes); > >> return; > >> } > >> > >> diff --git a/core/progress_thread.c b/core/progress_thread.c > >> index cc556c1..7c424fc 100644 > >> --- a/core/progress_thread.c > >> +++ b/core/progress_thread.c > >> @@ -97,7 +97,7 @@ static void _swupdate_download_update(unsigned int perc, unsigned long long tota > >> pthread_mutex_lock(&pprog->lock); > >> if (perc != pprog->msg.dwl_percent) { > >> pprog->msg.dwl_percent = perc; > >> - totalbytes = totalbytes; > >> + pprog->msg.dwl_bytes = totalbytes; > >> send_progress_msg(); > >> } > >> pthread_mutex_unlock(&pprog->lock); > >> @@ -138,8 +138,13 @@ void swupdate_download_update(unsigned int perc, unsigned long long totalbytes) > >> * Not called by main process, for example by suricatta or Webserver > >> */ > >> if (pid == getpid()) { > >> - struct progress_dwl_data *pdwl = (struct progress_dwl_data *)info; > >> - pdwl->dwl_percent = perc; > >> + /* > >> + * Notify can just receive a string as message > >> + * so it is necessary to encode further information as string > >> + * and decode them in the notifier, in this case > >> + * the progress_notifier > >> + */ > >> + snprintf(info, sizeof(info) - 1, "%d-%llu", perc, totalbytes); > >> notify(PROGRESS, RECOVERY_DWL, TRACELEVEL, info); > >> return; > >> } > >> diff --git a/include/progress.h b/include/progress.h > >> index 738e59f..9297cf9 100644 > >> --- a/include/progress.h > >> +++ b/include/progress.h > >> @@ -11,15 +11,6 @@ > >> #include <swupdate_status.h> > >> #include <progress_ipc.h> > >> > >> -/* > >> - * Internal structures to be used to forward progress data > >> - */ > >> - > >> -struct progress_dwl_data { > >> - unsigned int dwl_percent; /* % downloaded data */ > >> - unsigned long long dwl_bytes; /* total of bytes to be downloaded */ > >> -}; > >> - > >> /* > >> * Internal SWUpdate functions to drive the progress > >> * interface. Common progress definitions for internal > >> diff --git a/include/progress_ipc.h b/include/progress_ipc.h > >> index f508eda..2efa61d 100644 > >> --- a/include/progress_ipc.h > >> +++ b/include/progress_ipc.h > >> @@ -27,6 +27,7 @@ struct progress_msg { > >> unsigned int magic; /* Magic Number */ > >> RECOVERY_STATUS status; /* Update Status (Running, Failure) */ > >> unsigned int dwl_percent; /* % downloaded data */ > >> + unsigned long long dwl_bytes; /* total of bytes to be downloaded */ > >> unsigned int nsteps; /* No. total of steps */ > >> unsigned int cur_step; /* Current step index */ > >> unsigned int cur_percent; /* % in current step */ > >> diff --git a/tools/swupdate-progress.c b/tools/swupdate-progress.c > >> index ecc0b40..754d56f 100644 > >> --- a/tools/swupdate-progress.c > >> +++ b/tools/swupdate-progress.c > >> @@ -327,11 +327,11 @@ int main(int argc, char **argv) > >> fill_progress_bar(bar, sizeof(bar), msg.cur_percent); > >> > >> if (!silent) { > >> - fprintf(stdout, "[ %.*s ] %d of %d %d%% (%s) dwl %d%%\r", > >> + fprintf(stdout, "[ %.*s ] %d of %d %d%% (%s), dwl %d%% of %llu bytes\r", > >> bar_len, > >> bar, > >> msg.cur_step, msg.nsteps, msg.cur_percent, > >> - msg.cur_image, msg.dwl_percent); > >> + msg.cur_image, msg.dwl_percent, msg.dwl_bytes); > >> fflush(stdout); > >> } > >> > >> -- > >> 2.25.1 > >> > > > > Just found that this breaks core/syslog.c by it emitting messages like > > FATAL_UNKNOWN 15-217716224 > > with the message being the <percent>-<filesize> encoding. > > > > The 'FATAL_' prefix stems from this line in core/syslog.c > > syslog(logprio, "%s%s %s\n", ((error != (int)RECOVERY_NO_ERROR) ? "FATAL_" : ""), statusMsg, msg); > > where error == RECOVERY_DWL should also not result in the 'FATAL_' prefix. > > > > Then, > > switch(status) { > > should also cover > > case PROGRESS: statusMsg = "PROGRESS"; break; > > and probably also SUBPROCESS for completeness. > > > > This silences the FATAL alarm but still you get logged the encoded > > information in <percent>-<filesize> syntax. IMO, it should print > > something more friendly to human interpretation :) > > Well, but this generates an overflow of dummy messages for syslog > without meaning, as the message is addressed to the progress interface > instead of the logging. I will tend to not log unknown sources instead > ofadding them. So like: > > case DONE: statusMsg = "DONE"; break; > default: > return; Yes, suppressing those progress messages would also be an option, agreed. Kind regards, Christian
On 18.11.20 18:04, Christian Storm wrote: > Hi Stefano, > >>>> commit 5738732 disables the number of downloaded bytes because buggy, >>>> this fix the behavior and ensures that the value is correctly forwareded >>>> to the listeners. >>>> >>>> Signed-off-by: Stefano Babic <sbabic@denx.de> >>>> --- >>>> core/notifier.c | 7 ++++--- >>>> core/progress_thread.c | 11 ++++++++--- >>>> include/progress.h | 9 --------- >>>> include/progress_ipc.h | 1 + >>>> tools/swupdate-progress.c | 4 ++-- >>>> 5 files changed, 15 insertions(+), 17 deletions(-) >>>> >>>> diff --git a/core/notifier.c b/core/notifier.c >>>> index 276e155..e4ca3c6 100644 >>>> --- a/core/notifier.c >>>> +++ b/core/notifier.c >>>> @@ -339,15 +339,16 @@ static void process_notifier (RECOVERY_STATUS status, int event, int level, cons >>>> */ >>>> static void progress_notifier (RECOVERY_STATUS status, int event, int level, const char *msg) >>>> { >>>> + int dwl_percent = 0; >>>> + unsigned long long dwl_bytes = 0; >>>> (void)level; >>>> >>>> /* Check just in case a process want to send an info outside */ >>>> if (status != PROGRESS) >>>> return; >>>> >>>> - if (event == RECOVERY_DWL) { >>>> - struct progress_dwl_data *pdwl = (struct progress_dwl_data *)msg; >>>> - swupdate_download_update(pdwl->dwl_percent, pdwl->dwl_bytes); >>>> + if (event == RECOVERY_DWL && (sscanf(msg, "%d-%llu", &dwl_percent, &dwl_bytes) == 2)) { >>>> + swupdate_download_update(dwl_percent, dwl_bytes); >>>> return; >>>> } >>>> >>>> diff --git a/core/progress_thread.c b/core/progress_thread.c >>>> index cc556c1..7c424fc 100644 >>>> --- a/core/progress_thread.c >>>> +++ b/core/progress_thread.c >>>> @@ -97,7 +97,7 @@ static void _swupdate_download_update(unsigned int perc, unsigned long long tota >>>> pthread_mutex_lock(&pprog->lock); >>>> if (perc != pprog->msg.dwl_percent) { >>>> pprog->msg.dwl_percent = perc; >>>> - totalbytes = totalbytes; >>>> + pprog->msg.dwl_bytes = totalbytes; >>>> send_progress_msg(); >>>> } >>>> pthread_mutex_unlock(&pprog->lock); >>>> @@ -138,8 +138,13 @@ void swupdate_download_update(unsigned int perc, unsigned long long totalbytes) >>>> * Not called by main process, for example by suricatta or Webserver >>>> */ >>>> if (pid == getpid()) { >>>> - struct progress_dwl_data *pdwl = (struct progress_dwl_data *)info; >>>> - pdwl->dwl_percent = perc; >>>> + /* >>>> + * Notify can just receive a string as message >>>> + * so it is necessary to encode further information as string >>>> + * and decode them in the notifier, in this case >>>> + * the progress_notifier >>>> + */ >>>> + snprintf(info, sizeof(info) - 1, "%d-%llu", perc, totalbytes); >>>> notify(PROGRESS, RECOVERY_DWL, TRACELEVEL, info); >>>> return; >>>> } >>>> diff --git a/include/progress.h b/include/progress.h >>>> index 738e59f..9297cf9 100644 >>>> --- a/include/progress.h >>>> +++ b/include/progress.h >>>> @@ -11,15 +11,6 @@ >>>> #include <swupdate_status.h> >>>> #include <progress_ipc.h> >>>> >>>> -/* >>>> - * Internal structures to be used to forward progress data >>>> - */ >>>> - >>>> -struct progress_dwl_data { >>>> - unsigned int dwl_percent; /* % downloaded data */ >>>> - unsigned long long dwl_bytes; /* total of bytes to be downloaded */ >>>> -}; >>>> - >>>> /* >>>> * Internal SWUpdate functions to drive the progress >>>> * interface. Common progress definitions for internal >>>> diff --git a/include/progress_ipc.h b/include/progress_ipc.h >>>> index f508eda..2efa61d 100644 >>>> --- a/include/progress_ipc.h >>>> +++ b/include/progress_ipc.h >>>> @@ -27,6 +27,7 @@ struct progress_msg { >>>> unsigned int magic; /* Magic Number */ >>>> RECOVERY_STATUS status; /* Update Status (Running, Failure) */ >>>> unsigned int dwl_percent; /* % downloaded data */ >>>> + unsigned long long dwl_bytes; /* total of bytes to be downloaded */ >>>> unsigned int nsteps; /* No. total of steps */ >>>> unsigned int cur_step; /* Current step index */ >>>> unsigned int cur_percent; /* % in current step */ >>>> diff --git a/tools/swupdate-progress.c b/tools/swupdate-progress.c >>>> index ecc0b40..754d56f 100644 >>>> --- a/tools/swupdate-progress.c >>>> +++ b/tools/swupdate-progress.c >>>> @@ -327,11 +327,11 @@ int main(int argc, char **argv) >>>> fill_progress_bar(bar, sizeof(bar), msg.cur_percent); >>>> >>>> if (!silent) { >>>> - fprintf(stdout, "[ %.*s ] %d of %d %d%% (%s) dwl %d%%\r", >>>> + fprintf(stdout, "[ %.*s ] %d of %d %d%% (%s), dwl %d%% of %llu bytes\r", >>>> bar_len, >>>> bar, >>>> msg.cur_step, msg.nsteps, msg.cur_percent, >>>> - msg.cur_image, msg.dwl_percent); >>>> + msg.cur_image, msg.dwl_percent, msg.dwl_bytes); >>>> fflush(stdout); >>>> } >>>> >>>> -- >>>> 2.25.1 >>>> >>> >>> Just found that this breaks core/syslog.c by it emitting messages like >>> FATAL_UNKNOWN 15-217716224 >>> with the message being the <percent>-<filesize> encoding. >>> >>> The 'FATAL_' prefix stems from this line in core/syslog.c >>> syslog(logprio, "%s%s %s\n", ((error != (int)RECOVERY_NO_ERROR) ? "FATAL_" : ""), statusMsg, msg); >>> where error == RECOVERY_DWL should also not result in the 'FATAL_' prefix. >>> >>> Then, >>> switch(status) { >>> should also cover >>> case PROGRESS: statusMsg = "PROGRESS"; break; >>> and probably also SUBPROCESS for completeness. >>> >>> This silences the FATAL alarm but still you get logged the encoded >>> information in <percent>-<filesize> syntax. IMO, it should print >>> something more friendly to human interpretation :) >> >> Well, but this generates an overflow of dummy messages for syslog >> without meaning, as the message is addressed to the progress interface >> instead of the logging. I will tend to not log unknown sources instead >> ofadding them. So like: >> >> case DONE: statusMsg = "DONE"; break; >> default: >> return; > > Yes, suppressing those progress messages would also be an option, > agreed. > I send a patch... Stefano > > Kind regards, > Christian >
diff --git a/core/notifier.c b/core/notifier.c index 276e155..e4ca3c6 100644 --- a/core/notifier.c +++ b/core/notifier.c @@ -339,15 +339,16 @@ static void process_notifier (RECOVERY_STATUS status, int event, int level, cons */ static void progress_notifier (RECOVERY_STATUS status, int event, int level, const char *msg) { + int dwl_percent = 0; + unsigned long long dwl_bytes = 0; (void)level; /* Check just in case a process want to send an info outside */ if (status != PROGRESS) return; - if (event == RECOVERY_DWL) { - struct progress_dwl_data *pdwl = (struct progress_dwl_data *)msg; - swupdate_download_update(pdwl->dwl_percent, pdwl->dwl_bytes); + if (event == RECOVERY_DWL && (sscanf(msg, "%d-%llu", &dwl_percent, &dwl_bytes) == 2)) { + swupdate_download_update(dwl_percent, dwl_bytes); return; } diff --git a/core/progress_thread.c b/core/progress_thread.c index cc556c1..7c424fc 100644 --- a/core/progress_thread.c +++ b/core/progress_thread.c @@ -97,7 +97,7 @@ static void _swupdate_download_update(unsigned int perc, unsigned long long tota pthread_mutex_lock(&pprog->lock); if (perc != pprog->msg.dwl_percent) { pprog->msg.dwl_percent = perc; - totalbytes = totalbytes; + pprog->msg.dwl_bytes = totalbytes; send_progress_msg(); } pthread_mutex_unlock(&pprog->lock); @@ -138,8 +138,13 @@ void swupdate_download_update(unsigned int perc, unsigned long long totalbytes) * Not called by main process, for example by suricatta or Webserver */ if (pid == getpid()) { - struct progress_dwl_data *pdwl = (struct progress_dwl_data *)info; - pdwl->dwl_percent = perc; + /* + * Notify can just receive a string as message + * so it is necessary to encode further information as string + * and decode them in the notifier, in this case + * the progress_notifier + */ + snprintf(info, sizeof(info) - 1, "%d-%llu", perc, totalbytes); notify(PROGRESS, RECOVERY_DWL, TRACELEVEL, info); return; } diff --git a/include/progress.h b/include/progress.h index 738e59f..9297cf9 100644 --- a/include/progress.h +++ b/include/progress.h @@ -11,15 +11,6 @@ #include <swupdate_status.h> #include <progress_ipc.h> -/* - * Internal structures to be used to forward progress data - */ - -struct progress_dwl_data { - unsigned int dwl_percent; /* % downloaded data */ - unsigned long long dwl_bytes; /* total of bytes to be downloaded */ -}; - /* * Internal SWUpdate functions to drive the progress * interface. Common progress definitions for internal diff --git a/include/progress_ipc.h b/include/progress_ipc.h index f508eda..2efa61d 100644 --- a/include/progress_ipc.h +++ b/include/progress_ipc.h @@ -27,6 +27,7 @@ struct progress_msg { unsigned int magic; /* Magic Number */ RECOVERY_STATUS status; /* Update Status (Running, Failure) */ unsigned int dwl_percent; /* % downloaded data */ + unsigned long long dwl_bytes; /* total of bytes to be downloaded */ unsigned int nsteps; /* No. total of steps */ unsigned int cur_step; /* Current step index */ unsigned int cur_percent; /* % in current step */ diff --git a/tools/swupdate-progress.c b/tools/swupdate-progress.c index ecc0b40..754d56f 100644 --- a/tools/swupdate-progress.c +++ b/tools/swupdate-progress.c @@ -327,11 +327,11 @@ int main(int argc, char **argv) fill_progress_bar(bar, sizeof(bar), msg.cur_percent); if (!silent) { - fprintf(stdout, "[ %.*s ] %d of %d %d%% (%s) dwl %d%%\r", + fprintf(stdout, "[ %.*s ] %d of %d %d%% (%s), dwl %d%% of %llu bytes\r", bar_len, bar, msg.cur_step, msg.nsteps, msg.cur_percent, - msg.cur_image, msg.dwl_percent); + msg.cur_image, msg.dwl_percent, msg.dwl_bytes); fflush(stdout); }
commit 5738732 disables the number of downloaded bytes because buggy, this fix the behavior and ensures that the value is correctly forwareded to the listeners. Signed-off-by: Stefano Babic <sbabic@denx.de> --- core/notifier.c | 7 ++++--- core/progress_thread.c | 11 ++++++++--- include/progress.h | 9 --------- include/progress_ipc.h | 1 + tools/swupdate-progress.c | 4 ++-- 5 files changed, 15 insertions(+), 17 deletions(-)