search

ESP32-Musings / OTA_update_AVR_using_ESP32

Program AVR MCUs Over-the-Air using ESP32
MIT License
37 stars 13 forks source link

Mistype in adr_hi and adr_lo #2

Closed alambin closed 2 weeks ago

alambin commented 3 years ago

Hello. In function loadAddress() (master/components/avr_flash/avr_flash.c, line 16) looks like mistype in names of parameters. According to STK500 protocol description (http://www.amelek.gda.pl/avr/uisp/doc2525.pdf) after command ("0x55") first you should provide low address, then high address. So, total sequence should be "0x55, lo, hi". In function flashPage(), which calls loadAddress(), we are providing parameters also in incorrect order: address[1] is actually low part of address, but address[0] - high. Look at incrementLoadAddress() - first you increase loadAddress[1] and only after it overflows, you increase loadAddress[0]. So, one bug neutralized another. To write this code in proper way, we should swap parameters at lines 53 and 18.

It is just assumption, because I'm quite new in this protocol. Even if this is correct, it is more cosmetic change. Could you pleaes confirm that this assumption is correct? Or I'm mistaken?

laukik-hase commented 3 years ago

Hello, @alambin Sorry for the late reply.

Yes, you are correct. This can be modified for better readability. But, back when we were writing the code, we followed the principle - "If it works don't touch it". We will rectify it in some time. Thanks!