spanceac / PIC18-bootloader

A bootloader for the PIC18 MCUs
12 stars 5 forks source link

Problem in check_btld_overwrite with PIC18F46K22 #1

Closed nestor-alonso closed 1 week ago

nestor-alonso commented 2 weeks ago

Hello @spanceac,

First and foremost, thank you for your amazing job with this bootloader. I've tested several while trying to get a project running and yours was the best documented and the clearer to follow among all I've found.

I'm working on a bootloader for an already built device, based on PIC18F46K22. This is a 64kB flash device. I found an issue with the function check_btld_overwrite. It is not working right with addresses above 7F00. The problem seems to be with the sublw instruction, that is limited to dec(127).

I replaced that block of code with the following, that works with addresses up to FFFF. Please excuse that I'm not sending a proper patch for this.

flash_add_hi must be defined at the beginning of the asm file

check_btld_overwrite:
    movf record_type, w  
    btfss STATUS, Z      
    return                ; if record_type is not 0, return  
    movlw btld_code_start_hi-1  ; 0xFD-1 = 0xFC
    movwf btld_hi_addr    ; Store in f the high part of the bootloader start address
    movf flash_addr_hi, 0 ; Move the address to be tested to  W
    xorlw btld_code_start_hi-1 ; If  btld_code_start_hi =  flash_addr_hi then  Z = 1
    btfsc  STATUS, Z      ; Skip if Z is not 1 (we are not close to the bootloader, we are below or on top of the bootloader
    bra close_to_btld

    movf flash_addr_hi, 0 ; Reloads flash_addr_hi in W, since W was modified by the XORLW
    cpfsgt btld_hi_addr   ; Skip next instruction if  btld_code_start_hi is greater than W ( we are trying to write blow the bootloader -> No problem)
    bra addr_in_btld_zone ; If the address to be tested is greater than the bootloaders start address, the address is on top of the bootloader
    return                ; The address tested is below the bootloader's start address. No problem. Return.

I've also updated the Python code to Python 3.

import serial
import sys

def check_if_err(recv):
    if chr(recv[0]) == "G":
        print("Error: first instruction is not a GOTO, exiting")
        sys.exit(1)
    elif chr(recv[0]) == "X":
        print("Error: this hex overwrites the bootloader zone, exiting")
        sys.exit(1)
    elif chr(recv[0]) == "E":
        print("Error: MCU reported bad checksum, exiting")
        sys.exit(1)
    else:
        return

def main():
    recv = '1' # initialise char
    f = open(sys.argv[2], 'r')
    ser = serial.Serial(sys.argv[1], 115200, timeout = None)

    for line in f:
        print('Sending %s' % line,)
        ser.write(line.encode(),)
        print('Waiting for XON')
        recv = ser.read()
        while recv[0] != 17:
            print('We received 0x%s waiting for XON: %s' % (recv[0], chr(recv[0])))
            check_if_err(recv)
            recv = ser.read()
        print('Received XON ... continuing')

    print('Finished sending hex')
    ser.close()
    sys.exit(0)

if __name__ == "__main__":
    main()

Once again, thank you for your work!

spanceac commented 2 weeks ago

hi @nestor-alonso,

I'm surprised that someone actually uses this project, but I'm glad :smile:

And trying to check the sublw behavior I just found out that MPASM is not supported by Microchip anymore, thus I can't compile the project. This was another surprise for me, I tried to use the XC compiler to build the project but I haven't managed yet. I will let you know after I can actually run something in the simulator

nestor-alonso commented 2 weeks ago

Yes... sadly MPASM is not supported anymore. I'm using the last archived version: MPLab 8.92.

https://ww1.microchip.com/downloads/en/DeviceDoc/MPLAB_IDE_8_92.zip

I'm using it in Windows 10. I tried to move the code to XC but no luck either. We are still using 8.92 here for other things, so it work for us.

Thanks!

spanceac commented 1 week ago

hi @nestor-alonso ,

I found the problem. Can you please check the code in this PR https://github.com/spanceac/PIC18-bootloader/pull/2?

nestor-alonso commented 1 week ago

Done! It is working :100:

Tested on the actual hardware device.

Thank you!

spanceac commented 1 week ago

Fixed by https://github.com/spanceac/PIC18-bootloader/pull/2