| Message ID | 20211005140955.2013175-1-sava.jakovljev@teufel.de |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Suricatta: Add fast polling interval | expand |
On 05.10.21 16:09, Sava Jakovljev wrote: > * When in STATE_WAIT, when an installed upgrade is pending or when an > upgrade is being downloaded, use a different polling interval and > allow user to specify it using 'q' short and 'fast-polldelay' long option. > > Signed-off-by: Sava Jakovljev <sava.jakovljev@teufel.de> > --- > doc/source/swupdate.rst | 9 +++++++++ > suricatta/server_hawkbit.c | 41 ++++++++++++++++++++++++++------------ > suricatta/server_hawkbit.h | 1 + > 3 files changed, 38 insertions(+), 13 deletions(-) > > diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst > index a81b87a..2702879 100644 > --- a/doc/source/swupdate.rst > +++ b/doc/source/swupdate.rst > @@ -673,6 +673,15 @@ Mandatory arguments are marked with '\*': > | | | -n 2M : Set limit to 1 M/s. | > | | | -n 1G : Set limit to 1 G/s. | > +-------------------------+----------+--------------------------------------------+ > +| -q <seconds> | integer | Fast polling interval in seconds. | > +| | | This poll delay is used when the client is | > +| | | in state WAIT, when an upgrade is | > +| | | downloading or installed, but not yet | > +| | | activated. | > +| | | If this value is not specified by user, | > +| | | default value is used, which is the | > +| | | polling interval / 10. | > ++-------------------------+----------+--------------------------------------------+ > > > systemd Integration > diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c > index f8f560e..f4e72a8 100644 > --- a/suricatta/server_hawkbit.c > +++ b/suricatta/server_hawkbit.c > @@ -54,7 +54,8 @@ static struct option long_options[] = { > {"connection-timeout", required_argument, NULL, 's'}, > {"custom-http-header", required_argument, NULL, 'a'}, > {"max-download-speed", required_argument, NULL, 'l'}, > - {NULL, 0, NULL, 0}}; > + {"fast-polldelay", required_argument, NULL, 'q'}, > + {NULL, 0, NULL, 0}}; > > static unsigned short mandatory_argument_count = 0; > static pthread_mutex_t notifylock = PTHREAD_MUTEX_INITIALIZER; > @@ -112,6 +113,7 @@ static int get_target_data_length(void); > > server_hawkbit_t server_hawkbit = {.url = NULL, > .polling_interval = CHANNEL_DEFAULT_POLLING_INTERVAL, > + .fast_polling_interval = 0, // don't use fast polling by default. > .polling_interval_from_server = true, > .debug = false, > .device_id = NULL, > @@ -484,23 +486,27 @@ server_op_res_t server_set_polling_interval_json(json_object *json_root) > (unsigned int)(abs(timedate.tm_sec) + (abs(timedate.tm_min) * 60) + > (abs(timedate.tm_hour) * 60 * 60)); > > - /* > - * Generally, the server sets the poll interval > - * However, at the startup, it makes sense to speed up the process > - * If SWUpdate is in wait state, try faster. > - */ > - if (server_hawkbit.update_state == STATE_WAIT) > - polling_interval /= 10; > - > server_hawkbit.polling_interval = > polling_interval == 0 ? CHANNEL_DEFAULT_POLLING_INTERVAL : polling_interval; > + > DEBUG("Set polling interval to %ds as announced by server.", > server_hawkbit.polling_interval); > return SERVER_OK; > } > > +static inline unsigned int server_get_fast_polling_interval(void) > +{ > + return server_hawkbit.fast_polling_interval ? > + server_hawkbit.fast_polling_interval : > + server_hawkbit.polling_interval / 10; > +} > + > unsigned int server_get_polling_interval(void) > { > + if (server_hawkbit.update_state == STATE_WAIT || > + get_state() == STATE_INSTALLED) > + return server_get_fast_polling_interval(); > + > return server_hawkbit.polling_interval; > } > > @@ -667,7 +673,7 @@ static int server_check_during_dwl(void) > * if something on the server was changed and a cancel > * was requested > */ > - if ((now.tv_sec - server_time.tv_sec) < ((int)server_get_polling_interval())) > + if ((now.tv_sec - server_time.tv_sec) < ((int)server_get_fast_polling_interval())) > return 0; > > /* Update current server time */ > @@ -1615,7 +1621,9 @@ void server_print_help(void) > "\t -a, --custom-http-header <name> <value> Set custom HTTP header, " > "appended to every HTTP request being sent.\n" > "\t -n, --max-download-speed <limit> Set download speed limit.\n" > - "Example: -n 100k; -n 1M; -n 100; -n 1G\n", > + "Example: -n 100k; -n 1M; -n 100; -n 1G\n" > + "\t -q, --fast-polldelay <interval> Set polling interval in seconds." > + "It is used when client is in WAIT state, and when upgrade is installed and pending.", > CHANNEL_DEFAULT_POLLING_INTERVAL, CHANNEL_DEFAULT_RESUME_TRIES, > CHANNEL_DEFAULT_RESUME_DELAY, > INITIAL_STATUS_REPORT_WAIT_DELAY); > @@ -1644,6 +1652,9 @@ static int server_hawkbit_settings(void *elem, void __attribute__ ((__unused__) > get_field(LIBCFG_PARSER, elem, "polldelay", > &server_hawkbit.polling_interval); > > + get_field(LIBCFG_PARSER, elem, "fast-polldelay", > + &server_hawkbit.fast_polling_interval); > + > get_field(LIBCFG_PARSER, elem, "initial-report-resend-period", > &server_hawkbit.initial_report_resend_period); > > @@ -1706,8 +1717,8 @@ server_op_res_t server_start(char *fname, int argc, char *argv[]) > /* reset to optind=1 to parse suricatta's argument vector */ > optind = 1; > opterr = 0; > - while ((choice = getopt_long(argc, argv, "t:i:c:u:p:xr:y::w:k:g:f:2:m:s:a:n:", > - long_options, NULL)) != -1) { > + while ((choice = getopt_long(argc, argv, "t:i:c:u:p:xr:y::w:k:g:f:2:m:s:a:n:q:", > + long_options, NULL)) != -1) { > switch (choice) { > case 't': > SETSTRING(server_hawkbit.tenant, optarg); > @@ -1812,6 +1823,10 @@ server_op_res_t server_start(char *fname, int argc, char *argv[]) > channel_data_defaults.max_download_speed = > (unsigned int)ustrtoull(optarg, 10); > break; > + case 'q': > + server_hawkbit.fast_polling_interval = > + (unsigned int)strtoul(optarg, NULL, 10); > + break; > /* Ignore not recognized options, they can be already parsed by the caller */ > case '?': > break; > diff --git a/suricatta/server_hawkbit.h b/suricatta/server_hawkbit.h > index 67f1c31..85b3e2a 100644 > --- a/suricatta/server_hawkbit.h > +++ b/suricatta/server_hawkbit.h > @@ -26,6 +26,7 @@ typedef struct { > char *device_id; > char *tenant; > unsigned int polling_interval; > + unsigned int fast_polling_interval; > bool polling_interval_from_server; > bool debug; > struct dict configdata; > -- > 2.31.1 > Reviewed-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
Hi, > * When in STATE_WAIT, when an installed upgrade is pending or when an > upgrade is being downloaded, use a different polling interval and > allow user to specify it using 'q' short and 'fast-polldelay' long option. > > Signed-off-by: Sava Jakovljev <sava.jakovljev@teufel.de> > --- > doc/source/swupdate.rst | 9 +++++++++ > suricatta/server_hawkbit.c | 41 ++++++++++++++++++++++++++------------ > suricatta/server_hawkbit.h | 1 + > 3 files changed, 38 insertions(+), 13 deletions(-) > > diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst > index a81b87a..2702879 100644 > --- a/doc/source/swupdate.rst > +++ b/doc/source/swupdate.rst > @@ -673,6 +673,15 @@ Mandatory arguments are marked with '\*': > | | | -n 2M : Set limit to 1 M/s. | > | | | -n 1G : Set limit to 1 G/s. | > +-------------------------+----------+--------------------------------------------+ > +| -q <seconds> | integer | Fast polling interval in seconds. | > +| | | This poll delay is used when the client is | > +| | | in state WAIT, when an upgrade is | > +| | | downloading or installed, but not yet | > +| | | activated. | > +| | | If this value is not specified by user, | > +| | | default value is used, which is the | > +| | | polling interval / 10. | > ++-------------------------+----------+--------------------------------------------+ As this is specific to server_hawkbit.c, shouldn't this be noted in the docs as well? Suricatta servers may implement a subset of these or even totally different options. So, a footnote along the lines of "+ implemented by some suricatta modules, see `swupdate --help` for reference on what options are supported by yours." Another option would be to document the suricatta server options in separate sections per server implementation... Kind regards, Christian
Hello Christian, I agree. Would it be fine with you to add a "Hawkbit specific" note in the "Suricatta command line parameters" documentation section, and also to the output of "help" for swupdate/suricatta? I think it makes sense to do both, but wanna make sure there's nothing I'm missing. Best regards, Sava On Wednesday, October 6, 2021 at 4:19:22 PM UTC+2 Christian Storm wrote: > Hi, > > > * When in STATE_WAIT, when an installed upgrade is pending or when an > > upgrade is being downloaded, use a different polling interval and > > allow user to specify it using 'q' short and 'fast-polldelay' long > option. > > > > Signed-off-by: Sava Jakovljev <sava.ja...@teufel.de> > > --- > > doc/source/swupdate.rst | 9 +++++++++ > > suricatta/server_hawkbit.c | 41 ++++++++++++++++++++++++++------------ > > suricatta/server_hawkbit.h | 1 + > > 3 files changed, 38 insertions(+), 13 deletions(-) > > > > diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst > > index a81b87a..2702879 100644 > > --- a/doc/source/swupdate.rst > > +++ b/doc/source/swupdate.rst > > @@ -673,6 +673,15 @@ Mandatory arguments are marked with '\*': > > | | | -n 2M : Set limit to 1 M/s. | > > | | | -n 1G : Set limit to 1 G/s. | > > > +-------------------------+----------+--------------------------------------------+ > > +| -q <seconds> | integer | Fast polling interval in seconds. | > > +| | | This poll delay is used when the client is | > > +| | | in state WAIT, when an upgrade is | > > +| | | downloading or installed, but not yet | > > +| | | activated. | > > +| | | If this value is not specified by user, | > > +| | | default value is used, which is the | > > +| | | polling interval / 10. | > > > ++-------------------------+----------+--------------------------------------------+ > > As this is specific to server_hawkbit.c, shouldn't this be noted in the > docs as well? Suricatta servers may implement a subset of these or even > totally different options. So, a footnote along the lines of > "+ implemented by some suricatta modules, see `swupdate --help` for > reference on what options are supported by yours." > > Another option would be to document the suricatta server options in > separate > sections per server implementation... > > > Kind regards, > Christian > > -- > Dr. Christian Storm > Siemens AG, Technology, T RDA IOT SES-DE > Otto-Hahn-Ring 6, 81739 München, Germany >
Hi, > > > * When in STATE_WAIT, when an installed upgrade is pending or when an > > > upgrade is being downloaded, use a different polling interval and > > > allow user to specify it using 'q' short and 'fast-polldelay' long > > option. > > > > > > Signed-off-by: Sava Jakovljev <sava.ja...@teufel.de> > > > --- > > > doc/source/swupdate.rst | 9 +++++++++ > > > suricatta/server_hawkbit.c | 41 ++++++++++++++++++++++++++------------ > > > suricatta/server_hawkbit.h | 1 + > > > 3 files changed, 38 insertions(+), 13 deletions(-) > > > > > > diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst > > > index a81b87a..2702879 100644 > > > --- a/doc/source/swupdate.rst > > > +++ b/doc/source/swupdate.rst > > > @@ -673,6 +673,15 @@ Mandatory arguments are marked with '\*': > > > | | | -n 2M : Set limit to 1 M/s. | > > > | | | -n 1G : Set limit to 1 G/s. | > > > > > +-------------------------+----------+--------------------------------------------+ > > > +| -q <seconds> | integer | Fast polling interval in seconds. | > > > +| | | This poll delay is used when the client is | > > > +| | | in state WAIT, when an upgrade is | > > > +| | | downloading or installed, but not yet | > > > +| | | activated. | > > > +| | | If this value is not specified by user, | > > > +| | | default value is used, which is the | > > > +| | | polling interval / 10. | > > > > > ++-------------------------+----------+--------------------------------------------+ > > > > As this is specific to server_hawkbit.c, shouldn't this be noted in the > > docs as well? Suricatta servers may implement a subset of these or even > > totally different options. So, a footnote along the lines of > > "+ implemented by some suricatta modules, see `swupdate --help` for > > reference on what options are supported by yours." > > > > Another option would be to document the suricatta server options in > > separate > > sections per server implementation... > > > I agree. > Would it be fine with you to add a "Hawkbit specific" note in the > "Suricatta command line parameters" documentation section, and also to the > output of "help" for swupdate/suricatta? > I think it makes sense to do both, but wanna make sure there's nothing I'm > missing. As we currently have exactly one suricatta "module" compiled-in/running, I think the output from swupdate --help is quite clear (and comes from that module), so there it can be skipped (for now) I guess. A different one is the documentation. As more specific options are added per suricatta module, things get confusing and hard to read. So, I'm in favor of adding a section per suricatta module to the documentation that documents its parameters, i.e., one for hawkBit, one for the general one, so we can add sections for suricatta modules to come easily and don't have to mess with footnotes or the like... Stefano, what's your take on this? Kind regards, Christian
Hi Christian, Sava, On 11.10.21 20:34, Christian Storm wrote: > Hi, > > >>>> * When in STATE_WAIT, when an installed upgrade is pending or when an >>>> upgrade is being downloaded, use a different polling interval and >>>> allow user to specify it using 'q' short and 'fast-polldelay' long >>> option. >>>> >>>> Signed-off-by: Sava Jakovljev <sava.ja...@teufel.de> >>>> --- >>>> doc/source/swupdate.rst | 9 +++++++++ >>>> suricatta/server_hawkbit.c | 41 ++++++++++++++++++++++++++------------ >>>> suricatta/server_hawkbit.h | 1 + >>>> 3 files changed, 38 insertions(+), 13 deletions(-) >>>> >>>> diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst >>>> index a81b87a..2702879 100644 >>>> --- a/doc/source/swupdate.rst >>>> +++ b/doc/source/swupdate.rst >>>> @@ -673,6 +673,15 @@ Mandatory arguments are marked with '\*': >>>> | | | -n 2M : Set limit to 1 M/s. | >>>> | | | -n 1G : Set limit to 1 G/s. | >>>> >>> +-------------------------+----------+--------------------------------------------+ >>>> +| -q <seconds> | integer | Fast polling interval in seconds. | >>>> +| | | This poll delay is used when the client is | >>>> +| | | in state WAIT, when an upgrade is | >>>> +| | | downloading or installed, but not yet | >>>> +| | | activated. | >>>> +| | | If this value is not specified by user, | >>>> +| | | default value is used, which is the | >>>> +| | | polling interval / 10. | >>>> >>> ++-------------------------+----------+--------------------------------------------+ >>> >>> As this is specific to server_hawkbit.c, shouldn't this be noted in the >>> docs as well? Suricatta servers may implement a subset of these or even >>> totally different options. So, a footnote along the lines of >>> "+ implemented by some suricatta modules, see `swupdate --help` for >>> reference on what options are supported by yours." >>> >>> Another option would be to document the suricatta server options in >>> separate >>> sections per server implementation... >>> >> I agree. >> Would it be fine with you to add a "Hawkbit specific" note in the >> "Suricatta command line parameters" documentation section, and also to the >> output of "help" for swupdate/suricatta? >> I think it makes sense to do both, but wanna make sure there's nothing I'm >> missing. > > As we currently have exactly one suricatta "module" compiled-in/running, > I think the output from swupdate --help is quite clear (and comes from > that module), so there it can be skipped (for now) I guess. Fully agree - I even thought to enable at runtime more suricatta modules, but frankly speaking this does not seen at the moment a use case. Then swupdate --help gives exactly the help for the compiled module and it is fully ok. > > A different one is the documentation. As more specific options are > added per suricatta module, things get confusing and hard to read. > So, I'm in favor of adding a section per suricatta module to the > documentation that documents its parameters, i.e., one for hawkBit, > one for the general one, so we can add sections for suricatta modules > to come easily and don't have to mess with footnotes or the like... +1 We can have a "common suricatta parameters", that is only enable / disable, and then the description of parameters for each module, that is currently hawkbit and general. It is also fine with me if at the moment just the Hawkbit section is added, and I can add later the section for the generic server, that is not so used as Hawkbit. > > > Stefano, what's your take on this? I like your approach. Best regards, Stefano > > > > > Kind regards, > Christian >
diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst index a81b87a..2702879 100644 --- a/doc/source/swupdate.rst +++ b/doc/source/swupdate.rst @@ -673,6 +673,15 @@ Mandatory arguments are marked with '\*': | | | -n 2M : Set limit to 1 M/s. | | | | -n 1G : Set limit to 1 G/s. | +-------------------------+----------+--------------------------------------------+ +| -q <seconds> | integer | Fast polling interval in seconds. | +| | | This poll delay is used when the client is | +| | | in state WAIT, when an upgrade is | +| | | downloading or installed, but not yet | +| | | activated. | +| | | If this value is not specified by user, | +| | | default value is used, which is the | +| | | polling interval / 10. | ++-------------------------+----------+--------------------------------------------+ systemd Integration diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c index f8f560e..f4e72a8 100644 --- a/suricatta/server_hawkbit.c +++ b/suricatta/server_hawkbit.c @@ -54,7 +54,8 @@ static struct option long_options[] = { {"connection-timeout", required_argument, NULL, 's'}, {"custom-http-header", required_argument, NULL, 'a'}, {"max-download-speed", required_argument, NULL, 'l'}, - {NULL, 0, NULL, 0}}; + {"fast-polldelay", required_argument, NULL, 'q'}, + {NULL, 0, NULL, 0}}; static unsigned short mandatory_argument_count = 0; static pthread_mutex_t notifylock = PTHREAD_MUTEX_INITIALIZER; @@ -112,6 +113,7 @@ static int get_target_data_length(void); server_hawkbit_t server_hawkbit = {.url = NULL, .polling_interval = CHANNEL_DEFAULT_POLLING_INTERVAL, + .fast_polling_interval = 0, // don't use fast polling by default. .polling_interval_from_server = true, .debug = false, .device_id = NULL, @@ -484,23 +486,27 @@ server_op_res_t server_set_polling_interval_json(json_object *json_root) (unsigned int)(abs(timedate.tm_sec) + (abs(timedate.tm_min) * 60) + (abs(timedate.tm_hour) * 60 * 60)); - /* - * Generally, the server sets the poll interval - * However, at the startup, it makes sense to speed up the process - * If SWUpdate is in wait state, try faster. - */ - if (server_hawkbit.update_state == STATE_WAIT) - polling_interval /= 10; - server_hawkbit.polling_interval = polling_interval == 0 ? CHANNEL_DEFAULT_POLLING_INTERVAL : polling_interval; + DEBUG("Set polling interval to %ds as announced by server.", server_hawkbit.polling_interval); return SERVER_OK; } +static inline unsigned int server_get_fast_polling_interval(void) +{ + return server_hawkbit.fast_polling_interval ? + server_hawkbit.fast_polling_interval : + server_hawkbit.polling_interval / 10; +} + unsigned int server_get_polling_interval(void) { + if (server_hawkbit.update_state == STATE_WAIT || + get_state() == STATE_INSTALLED) + return server_get_fast_polling_interval(); + return server_hawkbit.polling_interval; } @@ -667,7 +673,7 @@ static int server_check_during_dwl(void) * if something on the server was changed and a cancel * was requested */ - if ((now.tv_sec - server_time.tv_sec) < ((int)server_get_polling_interval())) + if ((now.tv_sec - server_time.tv_sec) < ((int)server_get_fast_polling_interval())) return 0; /* Update current server time */ @@ -1615,7 +1621,9 @@ void server_print_help(void) "\t -a, --custom-http-header <name> <value> Set custom HTTP header, " "appended to every HTTP request being sent.\n" "\t -n, --max-download-speed <limit> Set download speed limit.\n" - "Example: -n 100k; -n 1M; -n 100; -n 1G\n", + "Example: -n 100k; -n 1M; -n 100; -n 1G\n" + "\t -q, --fast-polldelay <interval> Set polling interval in seconds." + "It is used when client is in WAIT state, and when upgrade is installed and pending.", CHANNEL_DEFAULT_POLLING_INTERVAL, CHANNEL_DEFAULT_RESUME_TRIES, CHANNEL_DEFAULT_RESUME_DELAY, INITIAL_STATUS_REPORT_WAIT_DELAY); @@ -1644,6 +1652,9 @@ static int server_hawkbit_settings(void *elem, void __attribute__ ((__unused__) get_field(LIBCFG_PARSER, elem, "polldelay", &server_hawkbit.polling_interval); + get_field(LIBCFG_PARSER, elem, "fast-polldelay", + &server_hawkbit.fast_polling_interval); + get_field(LIBCFG_PARSER, elem, "initial-report-resend-period", &server_hawkbit.initial_report_resend_period); @@ -1706,8 +1717,8 @@ server_op_res_t server_start(char *fname, int argc, char *argv[]) /* reset to optind=1 to parse suricatta's argument vector */ optind = 1; opterr = 0; - while ((choice = getopt_long(argc, argv, "t:i:c:u:p:xr:y::w:k:g:f:2:m:s:a:n:", - long_options, NULL)) != -1) { + while ((choice = getopt_long(argc, argv, "t:i:c:u:p:xr:y::w:k:g:f:2:m:s:a:n:q:", + long_options, NULL)) != -1) { switch (choice) { case 't': SETSTRING(server_hawkbit.tenant, optarg); @@ -1812,6 +1823,10 @@ server_op_res_t server_start(char *fname, int argc, char *argv[]) channel_data_defaults.max_download_speed = (unsigned int)ustrtoull(optarg, 10); break; + case 'q': + server_hawkbit.fast_polling_interval = + (unsigned int)strtoul(optarg, NULL, 10); + break; /* Ignore not recognized options, they can be already parsed by the caller */ case '?': break; diff --git a/suricatta/server_hawkbit.h b/suricatta/server_hawkbit.h index 67f1c31..85b3e2a 100644 --- a/suricatta/server_hawkbit.h +++ b/suricatta/server_hawkbit.h @@ -26,6 +26,7 @@ typedef struct { char *device_id; char *tenant; unsigned int polling_interval; + unsigned int fast_polling_interval; bool polling_interval_from_server; bool debug; struct dict configdata;
* When in STATE_WAIT, when an installed upgrade is pending or when an upgrade is being downloaded, use a different polling interval and allow user to specify it using 'q' short and 'fast-polldelay' long option. Signed-off-by: Sava Jakovljev <sava.jakovljev@teufel.de> --- doc/source/swupdate.rst | 9 +++++++++ suricatta/server_hawkbit.c | 41 ++++++++++++++++++++++++++------------ suricatta/server_hawkbit.h | 1 + 3 files changed, 38 insertions(+), 13 deletions(-) -- 2.31.1