Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot flash due to "Problem reading Hex file" #3

Closed
ansemjo opened this issue Feb 21, 2020 · 3 comments · Fixed by #4
Closed

Cannot flash due to "Problem reading Hex file" #3

ansemjo opened this issue Feb 21, 2020 · 3 comments · Fixed by #4

Comments

@ansemjo
Copy link
Contributor

ansemjo commented Feb 21, 2020

Description

I'm trying to flash a simple blink example to an ATmega4809, which was compiled with PlatformIO. updiprog cannot read the hex file, however.

After adding a few debug statements, the culprit appears to be the sanity check in IHEX_ReadFile:

updiprog/ihex.c

Lines 148 to 149 in 3741ea7

if (len * 2 + IHEX_MIN_STRING != strlen(str))
return IHEX_ERROR_FMT;

The left side returns 44 while strlen(str) returns 45.

If I just add another + 1 on the left side as an override, the program is flashed correctly and my LED blinks. Is strlen() handling the CRLF (see below) incorrectly?

I'm on Linux 5.5.4-arch1-1 x86_64.

Sources

src/blink.c:

#define F_CPU 3333333UL

#include <avr/io.h>
#include <util/delay.h>

#define LED 7

int main (void)
{
    PORTA.DIR |= (1 << LED);

    while (1)
    {
        PORTA.OUT ^= (1 << LED);
        _delay_ms(500);
    }
}

.pio/build/mega/firmware.hex:

:100000000C9450000C945A000C945A000C945A0012
:100010000C945A000C945A000C945A000C945A00F8
:100020000C945A000C945A000C945A000C945A00E8
:100030000C945A000C945A000C945A000C945A00D8
:100040000C945A000C945A000C945A000C945A00C8
:100050000C945A000C945A000C945A000C945A00B8
:100060000C945A000C945A000C945A000C945A00A8
:100070000C945A000C945A000C945A000C945A0098
:100080000C945A000C945A000C945A000C945A0088
:100090000C945A000C945A000C945A000C945A0078
:1000A00011241FBECFEFCDBFDFE3DEBF0E945C0097
:1000B0000C946E000C940000809100048068809382
:1000C00000048091040480588093040425E186E1B3
:1000D00095E0215080409040E1F7F3CFF894FFCFB6
:00000001FF

The hexdump shows that lines are terminated with \x0d\x0a, i.e. "CRLF":

$ xxd .pio/build/mega/firmware.hex | head -4
00000000: 3a31 3030 3030 3030 3030 4339 3435 3030  :100000000C94500
00000010: 3030 4339 3435 4130 3030 4339 3435 4130  00C945A000C945A0
00000020: 3030 4339 3435 4130 3031 320d 0a3a 3130  00C945A0012..:10  <<<
00000030: 3030 3130 3030 3043 3934 3541 3030 3043  0010000C945A000C
@ansemjo
Copy link
Contributor Author

ansemjo commented Feb 21, 2020

strlen(str) appears to be counting CRLF as two characters, therefore yielding an unexpected length. Completely trimming whitespace first and reducing the expected length by one fixes it for me and should be portable:

diff --git a/ihex.c b/ihex.c
index 860a8ad..642d54f 100644
--- a/ihex.c
+++ b/ihex.c
@@ -1,4 +1,5 @@
 #include <stdio.h>
+#include <ctype.h>
 #include <string.h>
 #include "ihex.h"
 
@@ -131,6 +132,7 @@ uint8_t IHEX_ReadFile(FILE *fp, uint8_t *data, uint16_t maxlen, uint16_t *max_ad
   uint8_t i;
   uint8_t byte;
   char str[128];
+  char *end;
 
   addr = 0;
   segment = 0;
@@ -138,6 +140,10 @@ uint8_t IHEX_ReadFile(FILE *fp, uint8_t *data, uint16_t maxlen, uint16_t *max_ad
   {
     if (fgets(str, sizeof(str), fp) == NULL)
       return IHEX_ERROR_FILE;
+    // trim whitespace on the right
+    end = str + strlen(str) - 1;
+    while (end > str && isspace((unsigned char) *end)) end--;
+    end[1] = '\0';
     if (strlen(str) < IHEX_MIN_STRING)
       return IHEX_ERROR_FMT;
     len = IHEX_GetByte(&str[IHEX_OFFS_LEN]);
diff --git a/ihex.h b/ihex.h
index d2f2f83..e96c6cf 100644
--- a/ihex.h
+++ b/ihex.h
@@ -6,7 +6,7 @@
 #include <stdbool.h>
 
 #define IHEX_LINE_LENGTH    16
-#define IHEX_MIN_STRING     12
+#define IHEX_MIN_STRING     11
 
 #define IHEX_OFFS_LEN       1
 #define IHEX_OFFS_ADDR      3

(taken from: https://stackoverflow.com/a/122721)

@Polarisru
Copy link
Owner

Hello, thank you for your message, i will check it with Windows version and commit the fix as soon as possible.

@ansemjo
Copy link
Contributor Author

ansemjo commented Feb 21, 2020

I've created a PR #4 for you with the above patch. :)

edit: attaching the source and firmware file if you need it for testing: blink.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants