diff --git a/Firmware/CHANGELOG.md b/Firmware/CHANGELOG.md index 0d6195e1..cb3b5b00 100644 --- a/Firmware/CHANGELOG.md +++ b/Firmware/CHANGELOG.md @@ -2,7 +2,11 @@ Please add a note of your changes below this heading if you make a Pull Request. ### Added +* `make write_otp` command to burn the board version onto the ODrive's one-time programmable memory. If you have an ODrive v3.4 or older, you can run this once for a better firmware update user experience in the future. Run the command without any options for more details. Once set, the board version is exposed through the `board_version_[...]` properties. +* bake Git-derived firmware version into firmware binary. The firmware version is exposed through the `fw_version_[...]` properties. + ### Changed +* The DFU script now verifies the flash after writing * Set thread priority of USB pump thread above protocol thread ### Fixed * Enums now transported with correct underlying type on native protocol diff --git a/Firmware/Makefile b/Firmware/Makefile index 84a28e30..e1d5fc09 100644 --- a/Firmware/Makefile +++ b/Firmware/Makefile @@ -28,6 +28,54 @@ bmp: all erase_config: openocd -f interface/stlink-v2.cfg -f target/stm32f4x.cfg -c init -c reset\ halt -c flash\ erase_address\ 0x80C0000\ 0x40000 -c reset\ run -c exit +# The one-time programmable memory stores the board version +# has the following format: +# - OTP format version (0xFE: version 1) +# - vendor ID (01: ODrive Robotics - do not use this on custom incompatible hardware!) +# - product ID (01: ODrive) +# - hardware major version +# - hardware minor version +# - hardware variant (equal to the board nominal voltage) +# Bits in the OTP can only ever be set to 0 but never back to 1. +# Therefore do not try to run this command on the same board +# twice with different data. +# +# This OpenOCD command is intended for a STM32F405 and does the following: +# FLASH_KEYR = 0x45670123; // unlock FLASH_CR +# FLASH_KEYR = 0xCDEF89AB; // unlock FLASH_CR +# FLASH_CR = (1 << FLASH_CR_PG); // unlock flash memory +# [write OTP] +write_otp: +ifeq ($(ODRV_FACTORY),TRUE) + # Data: + openocd -f interface/stlink-v2.cfg -f target/stm32f4x.cfg \ + -c init \ + -c 'reset halt' \ + -c 'mww 0x40023C04 0x45670123' \ + -c 'mww 0x40023C04 0xCDEF89AB' \ + -c 'mww 0x40023C10 0x00000001' -c 'sleep 10' \ + -c 'mwb 0x1fff7800 0xFE' -c 'sleep 10' \ + -c 'mwb 0x1fff7801 0x01' -c 'sleep 10' \ + -c 'mwb 0x1fff7802 0x01' -c 'sleep 10' \ + -c 'mwb 0x1fff7803 3' -c 'sleep 10' \ + -c 'mwb 0x1fff7804 4' -c 'sleep 10' \ + -c 'mwb 0x1fff7805 48' -c 'sleep 10' \ + -c 'reset run' \ + -c exit + +else + @echo "The one-time programmable memory can only be" + @echo "written ONCE on every board (what a surprise)." + @echo "If you're on an ODrive v3.5 or later we already did this for you." + @echo "Otherwise, if you're mentally ready for this irreversible action," + @echo "take the following steps:" + @echo " 1. open the Makefile and look at the write_otp target" + @echo " 2. understand the structure of the OTP" + @echo " 3. edit the bytes that are written to match your board version" + @echo "Run this command again, this time with ODRV_FACTORY=TRUE appended" + @echo "to the command in the terminal" +endif + clean: -rm -fR .dep $(BUILD_DIR) diff --git a/Firmware/MotorControl/commands.cpp b/Firmware/MotorControl/commands.cpp index 2cfb0387..4f8453d8 100644 --- a/Firmware/MotorControl/commands.cpp +++ b/Firmware/MotorControl/commands.cpp @@ -13,6 +13,7 @@ #include "freertos_vars.h" #include "utils.h" #include "config.h" +#include "../build/version.h" // autogenerated based on Git state #ifdef ENABLE_LEGACY_PROTOCOL #include "legacy_commands.h" @@ -113,12 +114,47 @@ void enter_dfu_mode() { NVIC_SystemReset(); } +#if HW_VERSION_MAJOR == 3 +// Determine start address of the OTP struct: +// The OTP is organized into 16-byte blocks. +// If the first block starts with "0xfe" we use the first block. +// If the first block starts with "0x00" and the second block starts with "0xfe", +// we use the second block. This gives the user the chance to screw up once. +// If none of the above is the case, we consider the OTP invalid (otp_ptr will be NULL). +const uint8_t* otp_ptr = + (*(uint8_t*)FLASH_OTP_BASE == 0xfe) ? (uint8_t*)FLASH_OTP_BASE : + (*(uint8_t*)FLASH_OTP_BASE != 0x00) ? NULL : + (*(uint8_t*)(FLASH_OTP_BASE + 0x10) != 0xfe) ? NULL : + (uint8_t*)(FLASH_OTP_BASE + 0x10); + +// Read hardware version from OTP if available, otherwise fall back +// to software defined version. +const uint8_t board_version_major = otp_ptr ? otp_ptr[3] : HW_VERSION_MAJOR; +const uint8_t board_version_minor = otp_ptr ? otp_ptr[4] : HW_VERSION_MINOR; +const uint8_t board_version_variant = otp_ptr ? otp_ptr[5] : HW_VERSION_VOLTAGE; +#else +#error "not implemented" +#endif + +// the corresponding macros are defined in the autogenerated version.h +const uint8_t fw_version_major = FW_VERSION_MAJOR; +const uint8_t fw_version_minor = FW_VERSION_MINOR; +const uint8_t fw_version_revision = FW_VERSION_REVISION; +const uint8_t fw_version_unreleased = FW_VERSION_UNRELEASED; // 0 for official releases, 1 otherwise + // This table specifies which fields and functions are exposed on the USB and UART ports. // TODO: Autogenerate this table. It will come up again very soon in the Arduino library. // clang-format off const Endpoint endpoints[] = { Endpoint::make_property("vbus_voltage", const_cast(&vbus_voltage)), Endpoint::make_property("serial_number", const_cast(&serial_number)), + Endpoint::make_property("board_version_major", &board_version_major), + Endpoint::make_property("board_version_minor", &board_version_minor), + Endpoint::make_property("board_version_variant", &board_version_variant), + Endpoint::make_property("fw_version_major", &fw_version_major), + Endpoint::make_property("fw_version_minor", &fw_version_minor), + Endpoint::make_property("fw_version_revision", &fw_version_revision), + Endpoint::make_property("fw_version_unreleased", &fw_version_unreleased), Endpoint::make_function("run_anticogging_calibration", &motors_run_anticogging_calibration_func), // No parameters, but still requires a close_tree() Endpoint::close_tree(), diff --git a/Firmware/Tupfile.lua b/Firmware/Tupfile.lua index 9d746830..0b5b263b 100644 --- a/Firmware/Tupfile.lua +++ b/Firmware/Tupfile.lua @@ -120,6 +120,11 @@ build{ includes=stm_includes } +tup.frule{ + command='bash dump_version.sh %o', + outputs={'build/version.h'} +} + build{ name='ODriveFirmware', toolchains={toolchain}, diff --git a/Firmware/build.lua b/Firmware/build.lua index ba5ecc30..898b357f 100644 --- a/Firmware/build.lua +++ b/Firmware/build.lua @@ -67,8 +67,9 @@ function GCCToolchain(prefix, builddir, compiler_flags, linker_flags) else extra_outputs = {} end + if src == 'MotorControl/commands.cpp' then extra_inputs = 'build/version.h' end -- TODO: fix hack tup.frule{ - inputs= { src }, + inputs= { src, extra_inputs=extra_inputs }, command=compiler..' -c %f '.. tostring(compiler_flags)..' '.. -- CFLAGS for this compiler tostring(inc_flags)..' '.. -- CFLAGS for this translation unit diff --git a/Firmware/dump_version.sh b/Firmware/dump_version.sh new file mode 100755 index 00000000..cdaaa372 --- /dev/null +++ b/Firmware/dump_version.sh @@ -0,0 +1,47 @@ +#!/bin/bash +set -euo pipefail + +if [ $# -eq 1 ]; then + OUTPUT="$1" +else + OUTPUT="/dev/stdout" +fi + +# The git root lies outside of the tup root +export GIT_DISCOVERY_ACROSS_FILESYSTEM=1 + +# Get a description of the current Git state +# Examples of what this string may become: +# fw-v0.3.6 The current commit is exactly at tag "fw-v0.3.6" +# There may or may not be untracked files in the +# working directory. +# fw-v0.3.6* The current commit is at tag "fw-v0.3.6" and there +# are uncommitted changes in the working directory. +# fw-v0.3.6-4-g3703ae5 The working directory at a commit with hash 3703ae5, +# 4 commits ahead of tag fw-v0.3.6 and clean. +FW_VERSION="$(git describe --always --tags --dirty=* || echo "[unknown commit]")" + +# Extract version numbers +FW_VERSION_MAJOR="$(sed -n 's/.*v\([0-9a-zA-Z]\).\([0-9a-zA-Z]\).\([0-9a-zA-Z]\)\(.*\)/\1/p' <<< "$FW_VERSION")" +FW_VERSION_MINOR="$(sed -n 's/.*v\([0-9a-zA-Z]\).\([0-9a-zA-Z]\).\([0-9a-zA-Z]\)\(.*\)/\2/p' <<< "$FW_VERSION")" +FW_VERSION_REVISION="$(sed -n 's/.*v\([0-9a-zA-Z]\).\([0-9a-zA-Z]\).\([0-9a-zA-Z]\)\(.*\)/\3/p' <<< "$FW_VERSION")" +FW_VERSION_SUFFIX="$(sed -n 's/.*v\([0-9a-zA-Z]\).\([0-9a-zA-Z]\).\([0-9a-zA-Z]\)\(.*\)/\4/p' <<< "$FW_VERSION")" + +# Fall back to 0 if the verions does not match the expected pattern +[ "$FW_VERSION_MAJOR" == "" ] && FW_VERSION_MAJOR=0 +[ "$FW_VERSION_MINOR" == "" ] && FW_VERSION_MINOR=0 +[ "$FW_VERSION_REVISION" == "" ] && FW_VERSION_REVISION=0 + +if [ "$FW_VERSION_SUFFIX" == "" ]; then + FW_VERSION_UNRELEASED=0 +else + FW_VERSION_UNRELEASED=1 +fi + +cat > "$OUTPUT" < 0: - # check if any segment from the hexfile overlaps with this sector - touched = False - for (start, end) in hexfile.segments(): - if start < addr and end > addr: - touched = True - break - elif start >= addr and start < addr + size: - touched = True - break - - if touched: - # TODO: verify if the section is writable - yield { - 'alt': alt, - 'addr': addr, - 'data': hexfile.tobinarray(addr, addr + size - 1) - } + # TODO: verify if the section is writable + yield { + 'name': label.strip().strip('@'), + 'alt': alt, + 'baseaddr': baseaddr, + 'addr': addr, + 'len': size, + 'mode': mode + } addr += size repeat -= 1 +def populate_sectors(sectors, hexfile): + """ + Checks for which on-device sectors there is data in the hex file and + returns a (sector, data) tuple for each touched sector where data + is a byte array of the same size as the sector. + """ + for sector in sectors: + addr = sector['addr'] + size = sector['len'] + # check if any segment from the hexfile overlaps with this sector + touched = False + for (start, end) in hexfile.segments(): + if start < addr and end > addr: + touched = True + break + elif start >= addr and start < addr + size: + touched = True + break + + if touched: + # TODO: verify if the section is writable + yield (sector, hexfile.tobinarray(addr, addr + size - 1)) + def set_alternate_safe(dfudev, alt): dfudev.set_alternate(alt) if dfudev.get_state() == dfuse.DfuState.DFU_ERROR: dfudev.clear_status() dfudev.wait_while_state(dfuse.DfuState.DFU_ERROR) -def erase(dfudev, sectors): - for i, sector in enumerate(sectors): - print("Erasing... (sector {}/{}) \r".format(i, len(sectors)), end='', flush=True) - set_alternate_safe(dfudev, sector['alt']) - dfudev.erase(sector['addr']) - status = dfudev.wait_while_state(dfuse.DfuState.DFU_DOWNLOAD_BUSY, timeout=len(sector['data'])/32) - if status[1] != dfuse.DfuState.DFU_DOWNLOAD_IDLE: - raise RuntimeError("An error occured. Device Status: %r" % status) - print('Erasing... done ') +#def clear_error(dfudev) +def set_address_safe(dfudef, addr): + dfudev.set_address(addr) + status = dfudev.wait_while_state(dfuse.DfuState.DFU_DOWNLOAD_BUSY) + if status[1] != dfuse.DfuState.DFU_DOWNLOAD_IDLE: + raise RuntimeError("An error occured. Device Status: %r" % status) + # take device out of DFU_DOWNLOAD_SYNC and into DFU_IDLE + dfudev.abort() + status = dfudev.wait_while_state(dfuse.DfuState.DFU_DOWNLOAD_SYNC) + if status[1] != dfuse.DfuState.DFU_IDLE: + raise RuntimeError("An error occured. Device Status: %r" % status) + -def flash(dfudev, sectors): - for i, sector in enumerate(sectors): - print("Flashing... (sector {}/{}) \r".format(i, len(sectors)), end='', flush=True) - set_alternate_safe(dfudev, sector['alt']) - dfudev.set_address(sector['addr']) +def erase(dfudev, sector): + set_alternate_safe(dfudev, sector['alt']) + dfudev.erase(sector['addr']) + status = dfudev.wait_while_state(dfuse.DfuState.DFU_DOWNLOAD_BUSY, timeout=sector['len']/32) + if status[1] != dfuse.DfuState.DFU_DOWNLOAD_IDLE: + raise RuntimeError("An error occured. Device Status: %r" % status) + +def flash(dfudev, sector, data): + set_alternate_safe(dfudev, sector['alt']) + set_address_safe(dfudev, sector['addr']) + + transfer_size = fractions.gcd(sector['len'], MAX_TRANSFER_SIZE) + + blocks = [data[i:i + transfer_size] for i in range(0, len(data), transfer_size)] + for blocknum, block in enumerate(blocks): + #print('write to {:08X} ({} bytes)'.format( + # sector['addr'] + blocknum * TRANSFER_SIZE, len(block))) + dfudev.write(blocknum, block) status = dfudev.wait_while_state(dfuse.DfuState.DFU_DOWNLOAD_BUSY) if status[1] != dfuse.DfuState.DFU_DOWNLOAD_IDLE: raise RuntimeError("An error occured. Device Status: %r" % status) - - data = sector['data'] - blocks = [data[i:i + TRANSFER_SIZE] for i in range(0, len(data), TRANSFER_SIZE)] - for blocknum, block in enumerate(blocks): - #print('write to {:08X} ({} bytes)'.format( - # sector['addr'] + blocknum * TRANSFER_SIZE, len(block))) - dfudev.write(blocknum, block) - status = dfudev.wait_while_state(dfuse.DfuState.DFU_DOWNLOAD_BUSY) - if status[1] != dfuse.DfuState.DFU_DOWNLOAD_IDLE: - raise RuntimeError("An error occured. Device Status: %r" % status) - print('Flashing... done ') +def read(dfudev, sector): + """ + Reads data from the specified sector + Returns: a byte array containing the data + """ + set_alternate_safe(dfudev, sector['alt']) + set_address_safe(dfudev, sector['addr']) -# Results in usb.core.USBError. Probably the device should go to dfuIDLE first, but how? -#def verify(dfudev, sectors): -# for i, sector in enumerate(sectors): -# print("Verifying... (sector {}/{}) \r".format(i, len(sectors)), end='', flush=True) -# set_alternate_safe(dfudev, sector['alt']) -# dfudev.set_address(sector['addr']) -# status = dfudev.wait_while_state(dfuse.DfuState.DFU_DOWNLOAD_BUSY) -# if status[1] != dfuse.DfuState.DFU_DOWNLOAD_IDLE: -# raise RuntimeError("An error occured. Device Status: %r" % status) -# -# print("state: {}".format(dfudev.get_state())) -# #dfudev.clear_status() -# print("state: {}".format(dfudev.get_state())) -# data = sector['data'] -# blocks = [data[i:i + TRANSFER_SIZE] for i in range(0, len(data), TRANSFER_SIZE)] -# for blocknum, block in enumerate(blocks): -# print('read at {:08X}'.format(sector['addr'] + blocknum * TRANSFER_SIZE)) -# deviceBlock = dfudev.read(blocknum, TRANSFER_SIZE) -# print(dfudev.get_state()) -# if (deviceBlock != block): -# raise RuntimeError("verification failed at address {:08X}".format(sector['addr'] + blocknum * TRANSFER_SIZE)) -# print('Verifying... done ') + transfer_size = fractions.gcd(sector['len'], MAX_TRANSFER_SIZE) + #blocknum_offset = int((sector['addr'] - sector['baseaddr']) / transfer_size) + + + data = array.array(u'B') + for blocknum in range(int(sector['len'] / transfer_size)): + #print('read at {:08X}'.format(sector['addr'] + blocknum * TRANSFER_SIZE)) + deviceBlock = dfudev.read(blocknum, transfer_size) + data.extend(deviceBlock) + dfudev.abort() # take device into DFU_IDLE + return data + +def get_first_mismatch_index(array1, array2): + """ + Compares two arrays and returns the index of the + first unequal item or None if both arrays are equal + """ + if len(array1) != len(array2): + raise Exception("arrays must be same size") + for pos in range(len(array1)): + if (array1[pos] != array2[pos]): + return pos + return None def jump_to_application(dfudev, address): - dfudev.set_address(address) - status = dfudev.wait_while_state(dfuse.DfuState.DFU_DOWNLOAD_BUSY) - if status[1] != dfuse.DfuState.DFU_DOWNLOAD_IDLE: - raise RuntimeError("An error occured. Device Status: {}".format(status[1])) + set_address_safe(dfudev, address) + #dfudev.set_address(address) + #status = dfudev.wait_while_state(dfuse.DfuState.DFU_DOWNLOAD_BUSY) + #if status[1] != dfuse.DfuState.DFU_DOWNLOAD_IDLE: + # raise RuntimeError("An error occured. Device Status: {}".format(status[1])) dfudev.leave() status = dfudev.wait_while_state(dfuse.DfuState.DFU_MANIFEST_SYNC) if status[1] != dfuse.DfuState.DFU_MANIFEST: raise RuntimeError("An error occured. Device Status: {}".format(status[1])) + +def dump_otp(): + """ + Dumps the contents of the one-time-programmable + memory. The OTP will be used in future versions of + this script to determine the board version. + """ + # 512 Byte OTP + otp_sector = [s for s in sectors if s['name'] == 'OTP Memory' and s['addr'] == 0x1fff7800][0] + data = read(dfudev, otp_sector) + print(' '.join('{:02X}'.format(x) for x in data)) + + # 16 lock bytes + otp_lock_sector = [s for s in sectors if s['name'] == 'OTP Memory' and s['addr'] == 0x1fff7A00][0] + data = read(dfudev, otp_lock_sector) + print(' '.join('{:02X}'.format(x) for x in data)) + def str_to_uuid(uuid): uuid = bytearray.fromhex(uuid.replace('-', '')) return struct.unpack('>I', uuid[0:4]), struct.unpack('>I', uuid[4:8]), struct.unpack('>I', uuid[8:12]) @@ -214,6 +266,8 @@ def put_odrive_into_dfu_mode_thread(cancellation_token): parser = argparse.ArgumentParser(description="Program an STM32 in DFU mode. The device can be identified either by it's serial number or UUID." "You can list all connected devices by running" "(lsusb -d 1209:0d32 -v; lsusb -d 0483:df11 -v) | grep iSerial") +parser.add_argument("-v", "--verbose", action="store_true", + help="print debug information") parser.add_argument('file', metavar='HEX', help='the .hex file to be flashed') parser.add_argument("-u", "--uuid", help="The 12-byte UUID of the device. This is a hexadecimal number of the format" @@ -260,17 +314,63 @@ try: dfudev = dfuse.DfuDevice(stm_device) + sectors = list(get_device_sectors(dfudev)) + + if (args.verbose): + print("Sectors on device: ") + for sector in sectors: + print(" {:08X} to {:08X} ({})".format( + sector['addr'], + sector['addr'] + sector['len'] - 1, + sector['name'])) # fill sectors with data - sectors = list(load_sectors(dfudev, hexfile)) - print("Sectors to be flashed: ") - for sector in sectors: - print(" {:08X} to {:08X}".format(sector['addr'], sector['addr'] + len(sector['data']) - 1)) + touched_sectors = list(populate_sectors(sectors, hexfile)) + + if (args.verbose): + print("The following sectors will be flashed: ") + for sector,_ in touched_sectors: + print(" {:08X} to {:08X}".format(sector['addr'], sector['addr'] + sector['len'] - 1)) + + if (args.verbose): + print("OTP:") + dump_otp() + + # Erase + try: + for i, (sector, data) in enumerate(touched_sectors): + print("Erasing... (sector {}/{}) \r".format(i, len(touched_sectors)), end='', flush=True) + erase(dfudev, sector) + print('Erasing... done \r', end='', flush=True) + finally: + print('', flush=True) + + # Flash + try: + for i, (sector, data) in enumerate(touched_sectors): + print("Flashing... (sector {}/{}) \r".format(i, len(touched_sectors)), end='', flush=True) + flash(dfudev, sector, data) + print('Flashing... done \r', end='', flush=True) + finally: + print('', flush=True) + + # Verify + try: + for i, (sector, expected_data) in enumerate(touched_sectors): + print("Verifying... (sector {}/{}) \r".format(i, len(touched_sectors)), end='', flush=True) + observed_data = read(dfudev, sector) + mismatch_pos = get_first_mismatch_index(observed_data, expected_data) + if not mismatch_pos is None: + mismatch_pos -= mismatch_pos % 16 + observed_snippet = ' '.join('{:02X}'.format(x) for x in observed_data[mismatch_pos:mismatch_pos+16]) + expected_snippet = ' '.join('{:02X}'.format(x) for x in expected_data[mismatch_pos:mismatch_pos+16]) + raise RuntimeError("Verification failed around address 0x{:08X}:\n".format(sector['addr'] + mismatch_pos) + + " expected: " + expected_snippet + "\n" + " observed: " + observed_snippet) + print('Verifying... done \r', end='', flush=True) + finally: + print('', flush=True) - # flash! - erase(dfudev, sectors) - flash(dfudev, sectors) - #verify(dfudev, sectors) # If the flash operation failed for some reason, your device is bricked now. # You can unbrick it as long as the device remains powered on. diff --git a/tools/dfuse/DfuDevice.py b/tools/dfuse/DfuDevice.py index 9173dcc4..dc5ac152 100644 --- a/tools/dfuse/DfuDevice.py +++ b/tools/dfuse/DfuDevice.py @@ -59,6 +59,9 @@ class DfuDevice: def get_state(self): return self.control_msg(DFU_REQUEST_RECEIVE, DFU_GETSTATE, 0, 1)[0] + def abort(self): + self.control_msg(DFU_REQUEST_RECEIVE, DFU_ABORT, 0, 0) + def set_address(self, ap): return self.dnload(0x0, [0x21] + address_to_4bytes(ap))