On Tue, 5 Feb 2013, Phil Dibowitz wrote:
Also, what do you think about moving the pre/post web function calls into
update_firmware/config()?
In case you were having trouble visualizing :), the attached is what I was
thinking. I think it makes the API just a little bit cleaner - all of the
functionality related to updating the config or firmware can be done in a
single call.
I don't see the benefit here. They're pretty discreet functions, and while
it's common to want to do them together, it's clearly not always the case.
Passing these configurations into an API on whether or not to call another API
seems more cumbersome than just letting the user call or not call that
additional API.
OK, I guess I'll back off on that piece. I left the set_time() move in
because I think that should at least be consistent between update_config()
and update_firmware(), unless you have a specific reason for that.
(update_firmware currently does a set_time whereas update_config does not.
In the update_config case, concordance was calling set_time directly.)
I also rewrote some of the get_stages stuff to be a bit more dynamic, rather
than have a bunch of arrays that are hard coded.
LOVE THIS.
Updated in attached v3. Note that the MH and this patch are touching the
same area of code, so depending on which one gets merged first, the other
will need minor tweaking.Move set_time() functionality inside of update_configuration and call the
internal version so that _report_stages() does not get called. Rewrite
get_stages functionality to be more dynamically generated rather than lots of
hard coded arrays.
Signed-off-by: Scott Talbert <s...@techie.net>
Index: concordance/concordance.c
===================================================================
RCS file: /cvsroot/concordance/concordance/concordance/concordance.c,v
retrieving revision 1.41.2.23
diff -u -p -r1.41.2.23 concordance.c
--- concordance/concordance.c 12 Jan 2013 23:50:32 -0000 1.41.2.23
+++ concordance/concordance.c 9 Feb 2013 18:22:12 -0000
@@ -807,11 +807,6 @@ int upload_config(struct options_t *opti
return err;
}
- /* Set the time, after a reboot */
- if ((err = set_time(cb, NULL))) {
- return err;
- }
-
/* Tell the website we're done */
if (!(*options).binary && !(*options).noweb) {
if ((err = post_postconfig(cb, cb_arg))) {
Index: libconcord/libconcord.cpp
===================================================================
RCS file: /cvsroot/concordance/concordance/libconcord/libconcord.cpp,v
retrieving revision 1.42.2.24
diff -u -p -r1.42.2.24 libconcord.cpp
--- libconcord/libconcord.cpp 12 Jan 2013 23:54:55 -0000 1.42.2.24
+++ libconcord/libconcord.cpp 9 Feb 2013 18:22:12 -0000
@@ -34,6 +34,7 @@
#include <zzip/lib.h>
#include <list>
#include <unistd.h>
+#include <vector>
#include "libconcord.h"
#include "lc_internal.h"
#include "remote.h"
@@ -601,33 +602,15 @@ static const uint32_t update_configurati
LC_CB_STAGE_ERASE_FLASH,
LC_CB_STAGE_WRITE_CONFIG,
LC_CB_STAGE_VERIFY_CONFIG,
- LC_CB_STAGE_RESET,
};
-static const int update_configuration_hid_num_stages = 6;
+static const int update_configuration_hid_num_stages = 5;
-static const uint32_t update_configuration_hid_noreset_stages[]={
- LC_CB_STAGE_INITIALIZE_UPDATE,
- LC_CB_STAGE_INVALIDATE_FLASH,
- LC_CB_STAGE_ERASE_FLASH,
- LC_CB_STAGE_WRITE_CONFIG,
- LC_CB_STAGE_VERIFY_CONFIG,
-};
-static const int update_configuration_hid_noreset_num_stages = 5;
-
-static const uint32_t update_configuration_zwave_stages[]={
+static const uint32_t update_configuration_zwave_mh_stages[]={
LC_CB_STAGE_INITIALIZE_UPDATE,
LC_CB_STAGE_WRITE_CONFIG,
LC_CB_STAGE_FINALIZE_UPDATE,
};
-static const int update_configuration_zwave_num_stages = 3;
-
-static const uint32_t update_configuration_usbnet_stages[]={
- LC_CB_STAGE_INITIALIZE_UPDATE,
- LC_CB_STAGE_WRITE_CONFIG,
- LC_CB_STAGE_FINALIZE_UPDATE,
- LC_CB_STAGE_RESET,
-};
-static const int update_configuration_usbnet_num_stages = 4;
+static const int update_configuration_zwave_mh_num_stages = 3;
static const uint32_t update_firmware_hid_stages[]={
LC_CB_STAGE_INITIALIZE_UPDATE,
@@ -635,19 +618,65 @@ static const uint32_t update_firmware_hi
LC_CB_STAGE_ERASE_FLASH,
LC_CB_STAGE_WRITE_FIRMWARE,
LC_CB_STAGE_FINALIZE_UPDATE,
- LC_CB_STAGE_RESET,
- LC_CB_STAGE_SET_TIME,
};
-static const int update_firmware_hid_num_stages = 7;
+static const int update_firmware_hid_num_stages = 5;
static const uint32_t update_firmware_hid_direct_stages[]={
LC_CB_STAGE_INVALIDATE_FLASH,
LC_CB_STAGE_ERASE_FLASH,
LC_CB_STAGE_WRITE_FIRMWARE,
- LC_CB_STAGE_RESET,
- LC_CB_STAGE_SET_TIME,
};
-static const int update_firmware_hid_direct_num_stages = 5;
+static const int update_firmware_hid_direct_num_stages = 3;
+
+std::vector<uint32_t> _get_update_config_stages(int noreset)
+{
+ std::vector<uint32_t> stages;
+ uint32_t *base_stages;
+ int num_base_stages;
+
+ if (is_z_remote()) {
+ base_stages = (uint32_t*)update_configuration_zwave_mh_stages;
+ num_base_stages = update_configuration_zwave_mh_num_stages;
+ } else {
+ base_stages = (uint32_t*)update_configuration_hid_stages;
+ num_base_stages = update_configuration_hid_num_stages;
+ }
+
+ for (int i = 0; i < num_base_stages; i++)
+ stages.push_back(base_stages[i]);
+
+ if (!noreset && !(is_z_remote() && !is_usbnet_remote()))
+ stages.push_back(LC_CB_STAGE_RESET);
+
+ stages.push_back(LC_CB_STAGE_SET_TIME);
+
+ return stages;
+}
+
+std::vector<uint32_t> _get_update_firmware_stages(int noreset, int direct)
+{
+ std::vector<uint32_t> stages;
+ uint32_t *base_stages;
+ int num_base_stages;
+
+ if (direct) {
+ base_stages = (uint32_t*)update_firmware_hid_direct_stages;
+ num_base_stages = update_firmware_hid_direct_num_stages;
+ } else {
+ base_stages = (uint32_t*)update_firmware_hid_stages;
+ num_base_stages = update_firmware_hid_num_stages;
+ }
+
+ for (int i = 0; i < num_base_stages; i++)
+ stages.push_back(base_stages[i]);
+
+ if (!noreset && !(is_z_remote() && !is_usbnet_remote()))
+ stages.push_back(LC_CB_STAGE_RESET);
+
+ stages.push_back(LC_CB_STAGE_SET_TIME);
+
+ return stages;
+}
/*
* GENERAL REMOTE STUFF
@@ -893,13 +922,14 @@ int get_time()
return 0;
}
-int _set_time(lc_callback cb, void *cb_arg, uint32_t cb_stage)
+int _set_time(lc_callback cb, void *cb_arg)
{
const time_t t = time(NULL);
struct tm *lt = localtime(&t);
if (cb)
- cb(cb_stage, 0, 1, 2, LC_CB_COUNTER_TYPE_STEPS, cb_arg, NULL);
+ cb(LC_CB_STAGE_SET_TIME, 0, 1, 2, LC_CB_COUNTER_TYPE_STEPS,
+ cb_arg, NULL);
rtime.second = lt->tm_sec;
rtime.minute = lt->tm_min;
@@ -916,7 +946,8 @@ int _set_time(lc_callback cb, void *cb_a
return err;
}
if (cb)
- cb(cb_stage, 1, 2, 2, LC_CB_COUNTER_TYPE_STEPS, cb_arg, NULL);
+ cb(LC_CB_STAGE_SET_TIME, 1, 2, 2, LC_CB_COUNTER_TYPE_STEPS,
+ cb_arg, NULL);
return 0;
}
@@ -924,7 +955,7 @@ int _set_time(lc_callback cb, void *cb_a
int set_time(lc_callback cb, void *cb_arg)
{
_report_stages(cb, cb_arg, 1, NULL);
- return _set_time(cb, cb_arg, LC_CB_STAGE_SET_TIME);
+ return _set_time(cb, cb_arg);
}
@@ -1150,25 +1181,13 @@ int _update_configuration_hid(lc_callbac
int update_configuration(lc_callback cb, void *cb_arg, int noreset)
{
int err;
+
+ std::vector<uint32_t> stages = _get_update_config_stages(noreset);
+ _report_stages(cb, cb_arg, stages.size(), &stages[0]);
+
if (is_z_remote()) {
- if (is_usbnet_remote())
- _report_stages(cb, cb_arg,
- update_configuration_usbnet_num_stages,
- update_configuration_usbnet_stages);
- else
- _report_stages(cb, cb_arg,
- update_configuration_zwave_num_stages,
- update_configuration_zwave_stages);
err = _update_configuration_zwave(cb, cb_arg);
} else {
- if (noreset)
- _report_stages(cb, cb_arg,
- update_configuration_hid_noreset_num_stages,
- update_configuration_hid_noreset_stages);
- else
- _report_stages(cb, cb_arg,
- update_configuration_hid_num_stages,
- update_configuration_hid_stages);
err = _update_configuration_hid(cb, cb_arg);
}
@@ -1176,10 +1195,11 @@ int update_configuration(lc_callback cb,
return err;
// usbnet seems to need a reset; not sure why zwave-hid's do not
- if (noreset || (is_z_remote() && !is_usbnet_remote()))
- return 0;
+ if (!noreset && !(is_z_remote() && !is_usbnet_remote()))
+ if ((err = reset_remote(cb, cb_arg)))
+ return err;
- if ((err = reset_remote(cb, cb_arg)))
+ if ((err = _set_time(cb, cb_arg)))
return err;
return 0;
@@ -1390,14 +1410,8 @@ int update_firmware(lc_callback cb, void
return LC_ERROR_UNSUPP;
}
- if (direct)
- _report_stages(cb, cb_arg,
- update_firmware_hid_direct_num_stages,
- update_firmware_hid_direct_stages);
- else
- _report_stages(cb, cb_arg,
- update_firmware_hid_num_stages,
- update_firmware_hid_stages);
+ vector<uint32_t> stages = _get_update_firmware_stages(noreset, direct);
+ _report_stages(cb, cb_arg, stages.size(), &stages[0]);
if (!direct) {
if ((err = prep_firmware(cb, cb_arg)))
@@ -1418,12 +1432,11 @@ int update_firmware(lc_callback cb, void
return err;
}
- if (noreset)
- return 0;
-
- reset_remote(cb, cb_arg);
+ if (!noreset)
+ if ((err = reset_remote(cb, cb_arg)))
+ return err;
- if ((err = set_time(cb, cb_arg)))
+ if ((err = _set_time(cb, cb_arg)))
return err;
return 0;
------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
concordance-devel mailing list
concordance-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/concordance-devel
