On Mon, 7 Jan 2013, Phil Dibowitz wrote:
- reset_remote(cb, cb_arg);
+ if ((err = reset_remote(cb, cb_arg)))
+ return err;
I think we did this because some remotes don't reset right and we didn't want
to return a failure because of it... we just try to reset them and if they
reset great and if not we set the time and move on.
Sadly, there's no comment, but I seem to recall that being the case... was
there a reason you changed this other than it seeming more correct?
I originally changed it to make it more consistent (we were checking the
return value from reset_remote() in update_configuration() but not in
update_firmware()). But subsequent to that, while investigating an issue
with a congruity user, I ran into the very issue you are describing -
where the reset command works, but we get a bad return from the
HID_WriteReport(). I was working on a patch to reset_remote where it
would ignore the specific error encountered in that case, but I haven't
yet been able to get feedback from the user on whether it works or not.
But I think it would be better solution to ignore the error inside of
reset_remote, rather than have it bubble up the higher layers. Do you
happen to have one of the remote models with the problematic reset (and
could test my patch)?
I don't have one of them (or rather, I don't think I do and I don't remember
what remotes they are).
I agree with you in theory, but we shouldn't cause a regression in this patch...
Fair enough, I reverted that change for now.
Implemented firmware updates for 688 remotes. Added capability to parse the
multi-phase firmware file that is used by the 688. Additionally, modified
the reset_remote logic to ignore "invalid config" as a fatal error.
Signed-off-by: Scott Talbert <s...@techie.net>
Index: libconcord/libconcord.cpp
===================================================================
RCS file: /cvsroot/concordance/concordance/libconcord/libconcord.cpp,v
retrieving revision 1.42.2.22
diff -u -p -r1.42.2.22 libconcord.cpp
--- libconcord/libconcord.cpp 15 Jul 2012 07:17:08 -0000 1.42.2.22
+++ libconcord/libconcord.cpp 8 Jan 2013 01:39:12 -0000
@@ -702,7 +702,15 @@ int reset_remote(lc_callback cb, void *c
err = init_concord();
if (err == 0) {
err = _get_identity(NULL, NULL, 0);
- if (err == 0) {
+ /*
+ * On remotes where firmware upgrades are not "direct",
+ * the config gets erased as part of the firmware
+ * update. Thus, the config could be invalid if we are
+ * resetting after a firmware upgrade, and we don't
+ * want to treat this as an error.
+ */
+ if ((err == 0) || (err == LC_ERROR_INVALID_CONFIG)) {
+ err = 0;
break;
}
deinit_concord();
Index: libconcord/operationfile.cpp
===================================================================
RCS file: /cvsroot/concordance/concordance/libconcord/Attic/operationfile.cpp,v
retrieving revision 1.1.2.3
diff -u -p -r1.1.2.3 operationfile.cpp
--- libconcord/operationfile.cpp 29 Sep 2012 23:51:38 -0000 1.1.2.3
+++ libconcord/operationfile.cpp 8 Jan 2013 01:39:12 -0000
@@ -198,10 +198,24 @@ int OperationFile::_ExtractFirmwareBinar
uint8_t *o = data;
uint8_t *x = xml;
+ uint8_t *x_new;
uint32_t x_size = xml_size;
+ /*
+ * Some remotes (e.g., Arch 7) contain multiple phases in their
+ * firmware update files. In that case, extract only the first phase.
+ */
+ if (GetTag("PHASE", x, x_size, x_new) == 0) {
+ debug("multi-phase firmware found, extracting 1st phase");
+ x_size = x_size - (x_new - x);
+ x = x_new;
+ uint8_t *phase_end;
+ GetTag("/PHASE", x, x_size, phase_end);
+ x_size = phase_end - x;
+ }
+
string hex;
- while (GetTag("DATA", x, x_size, x, &hex) == 0) {
+ while (GetTag("DATA", x, x_size, x_new, &hex) == 0) {
uint32_t hex_size = hex.length() / 2;
if (hex_size > o_size) {
return LC_ERROR;
@@ -209,7 +223,8 @@ int OperationFile::_ExtractFirmwareBinar
_convert_to_binary(hex, o);
- x_size = xml_size - (x - xml);
+ x_size = x_size - (x_new - x);
+ x = x_new;
o_size -= hex_size;
}
------------------------------------------------------------------------------
Master SQL Server Development, Administration, T-SQL, SSAS, SSIS, SSRS
and more. Get SQL Server skills now (including 2012) with LearnDevNow -
200+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
SALE $99.99 this month only - learn more at:
http://p.sf.net/sfu/learnmore_122512
_______________________________________________
concordance-devel mailing list
concordance-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/concordance-devel