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

Reply via email to