cnlohr / esp82xx

Useful ESP8266 C Environment
Other
288 stars 107 forks source link

Cannot upgrade via WEB [SOLVED, WITH PATCH] #61

Closed king2 closed 7 years ago

king2 commented 7 years ago

Just accurately merged new fresh downloaded lib into my firmware and found a problem: I cannot upgrade mpfs or firmware via web interface.

I have added some debug outputs to commonservices.c and can see that it receives strange data from web end. It succesfully parses commands (but strangely erased blocks in sequence of 8, 12, 13, 14..).

I have tried to see which data comes to spi_flash_write ('x' command) and found nothing that looks like my firmware files - and data received looks strange. It received as 99 00 33 00 00 66 00 00 44 00 00 11 00 44 00 00 00 00 00 11 00 44 00 66 cc 77 cc 00 00 00 00 00 instead of E9 03 00 60 04 00 10 40 00 00 10 40 6C 7C 00 00

When I have commented colon2++ (just after FLASH_PROTECTION_BOUNDARY check), I got my first 'E' back, so I think that or we do not getting a char from web, or we do not should skip this char from data received.

Next, I was wondered by 'r1 = r2 = fromhex1( *(colon2++) );' line. It assigns same values to both variables and check all these variables later one by one!

Hmmmm, maybe some compilers calls fromhex1 twice from this code?

I have de-composed this line to: r1 = fromhex1( (colon2++) ); r2 = fromhex1( (colon2++) );

And WOW! It started to work.

So, I have one question and one proposal.

  1. Why colon2 was incremented (and its value was skipped)? Maybe something wrong here?
  2. Please, return two calls to fromhex1 back! Some compilers with some optimization settings can raise headache to users in this place.

Thanks!

cnlohr commented 7 years ago

Do you mind making a pull request for this?

king2 commented 7 years ago

I'm not so familiar with all this github stuff, branches and so on :(

cnlohr commented 7 years ago

No time like the present :-p.

But really, if you're not up for learning, which file and which lines did you change?

king2 commented 7 years ago

commonservices.c, as it is was downloaded from github: line 630: comment or remove colon2++ (I still do not know its purpose) line 637: change 'r1 = r2 = ...' to: r1 = fromhex1((colon2++)); r2 = fromhex1((colon2++));

This is all.

con-f-use commented 7 years ago

Done in 96418251a dev branch - will propagate to master once everyone is happy with it). Thank you very much!

king2 commented 7 years ago

If I understand everything right, line 630 still was not removed/commented, this will cause it to skip first nibble of firmware (and OTA upgrade will fail at CRC calculating).

king2 commented 7 years ago

I have created pull request #62 that will fix this problem.

con-f-use commented 7 years ago

So the changes from your commit 9eff6b87 to master, namely

r1 = fromhex1((colon2++));
r2 = fromhex1((colon2++));

Were already in the dev branch as by yesterday's commit 9641825.