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

Incorrect reading from the s-record format files #162

Open
konstz43 opened this issue Apr 8, 2023 · 12 comments
Open

Incorrect reading from the s-record format files #162

konstz43 opened this issue Apr 8, 2023 · 12 comments

Comments

@konstz43
Copy link

konstz43 commented Apr 8, 2023

I tested stm8flash with ST-LINK/V2 and STM8S207CB

First I wrote to the MCU's flash memory area test_flash.bin and then successfully verified it.
Next I read flash memory into test_flash.s19 and then try to verify content of flash memory with the same test_flash.s19 but the verifing was failed.
When I wrote test_flash.s19 into the MCU's flash and verified it with the same file, program reported a successfull verification. But after this I verified MCU's flash with test_flash.bin and the verifing was failed.
Then I read MCU's flash into test_flash_1.bin and compare it with original test_flash.bin. I saw that the flash content in test_flash_1.bin was shifted by 1 byte.
This bug happens with Motorola S-record format and flash memory area only. With .bin and .hex formats the writing and verifying works fine. And with eeprom area all the formats work correctly. Maybe because the eeprom s-record file contains S1 records, whereas flash area file is S2.
It seems that the reason is the incorrect reading from the Motorola s-record format into memory for the files containing the flash area.

My files are attached.
My log is bellow:

$ stm8flash -c stlinkv2 -p stm8s207cb -s flash -w test_flash.bin
Determine FLASH area
STLink: v2, JTAG: v28, SWIM: v7, VID: 8304, PID: 4837
Due to its file extension (or lack thereof), "test_flash.bin" is considered as RAW BINARY format!
131072 bytes at 0x8000... OK
Bytes written: 131072

$ stm8flash -c stlinkv2 -p stm8s207cb -s flash -v test_flash.bin
Determine FLASH area
STLink: v2, JTAG: v28, SWIM: v7, VID: 8304, PID: 4837
Due to its file extension (or lack thereof), "test_flash.bin" is considered as RAW BINARY format!
Verifing 131072 bytes at 0x8000... OK
Bytes verified: 131072

$ stm8flash -c stlinkv2 -p stm8s207cb -s flash -r test_flash.s19
Determine FLASH area
STLink: v2, JTAG: v28, SWIM: v7, VID: 8304, PID: 4837
Due to its file extension (or lack thereof), "test_flash.s19" is considered as MOTOROLA S-RECORD format!
Reading 131072 bytes at 0x8000... OK
Bytes received: 131072

$ stm8flash -c stlinkv2 -p stm8s207cb -s flash -v test_flash.s19
Determine FLASH area
STLink: v2, JTAG: v28, SWIM: v7, VID: 8304, PID: 4837
Due to its file extension (or lack thereof), "test_flash.s19" is considered as MOTOROLA S-RECORD format!
Verifing 131072 bytes at 0x8000... FAILED

$ stm8flash -c stlinkv2 -p stm8s207cb -s flash -w test_flash.s19
Determine FLASH area
STLink: v2, JTAG: v28, SWIM: v7, VID: 8304, PID: 4837
Due to its file extension (or lack thereof), "test_flash.s19" is considered as MOTOROLA S-RECORD format!
131072 bytes at 0x8000... OK
Bytes written: 131072

$ stm8flash -c stlinkv2 -p stm8s207cb -s flash -v test_flash.s19
Determine FLASH area
STLink: v2, JTAG: v28, SWIM: v7, VID: 8304, PID: 4837
Due to its file extension (or lack thereof), "test_flash.s19" is considered as MOTOROLA S-RECORD format!
Verifing 131072 bytes at 0x8000... OK
Bytes verified: 131072

$ stm8flash -c stlinkv2 -p stm8s207cb -s flash -v test_flash.bin
Determine FLASH area
STLink: v2, JTAG: v28, SWIM: v7, VID: 8304, PID: 4837
Due to its file extension (or lack thereof), "test_flash.bin" is considered as RAW BINARY format!
Verifing 131072 bytes at 0x8000... FAILED

$ stm8flash -c stlinkv2 -p stm8s207cb -s flash -r test_flash_1.bin
Determine FLASH area
STLink: v2, JTAG: v28, SWIM: v7, VID: 8304, PID: 4837
Due to its file extension (or lack thereof), "test_flash_1.bin" is considered as RAW BINARY format!
Reading 131072 bytes at 0x8000... OK
Bytes received: 131072

test_flash.zip

@schneidersoft
Copy link
Contributor

There is a bug in how srec.c handles the records. with larger files the data being read is shifted by one.

@konstz43
Copy link
Author

Your pull request #164 solves this issue. I tested it. Thanks.

@konstz43
Copy link
Author

...but S1 records are processed incorrectly. The error Invalid checksum at line 2 occurs.
testS1.zip

@spth
Copy link
Collaborator

spth commented Apr 16, 2023

Is the incorrect processing of S1 records a regression in that pull request?

@konstz43
Copy link
Author

Yes, I ran gh pr checkout 164, then make and make install and then tested it.

@schneidersoft
Copy link
Contributor

Ill take another look at it later this week. In the meantime you can just use objcopy to convert your srec to ihex bofre feeding stm8flash, since srec support is broken.

@schneidersoft
Copy link
Contributor

I just rewrote the srec parser. It can handle both files you posted and should be much more robust than the previous implementation.

@konstz43
Copy link
Author

I tried to test it but got an error at the make stage:

~/stm8flash$ make
cc -g -O0 -DDEBUG=0 --std=gnu99 --pedantic -Wall `pkg-config --cflags libusb-1.0`    -c -o srec.o srec.c
srec.c: In function ‘load_srec’:
srec.c:92:31: error: ‘addrmin’ undeclared (first use in this function); did you mean ‘addrmax’?
   92 |  const unsigned int addrmax = addrmin + data_len;
      |                               ^~~~~~~
      |                               addrmax

@schneidersoft
Copy link
Contributor

appologies... fixed.

@konstz43
Copy link
Author

Thanks. No more error. I will test the functionality later today.

@konstz43
Copy link
Author

An error occurs when reading from srec file: address out of range on last line.
Files and log attached.
test_pr164.zip

@schneidersoft
Copy link
Contributor

schneidersoft commented Apr 20, 2023

nice catch. fixed. Also reading the verification code I can see that when the return of the srec_read function is not checked for failure. So if you ask for a verify of a corrupt srec file it will verify 0 bytes instead... super cool

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.

3 participants