From 0a02d824fc5003f8e01a17163c99b093a5d78d0f Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 22 Jun 2018 12:06:40 +1000 Subject: [PATCH] partition_table: Warn if the partition table name doesn't match type & subtype --- components/fatfs/test_fatfs_host/Makefile | 2 +- components/partition_table/gen_esp32part.py | 83 ++++++++++--------- .../gen_esp32part_tests.py | 39 +++++++-- components/spiffs/test_spiffs_host/Makefile | 2 +- .../wear_levelling/test_wl_host/Makefile | 2 +- docs/en/api-guides/partition-tables.rst | 6 +- 6 files changed, 82 insertions(+), 52 deletions(-) diff --git a/components/fatfs/test_fatfs_host/Makefile b/components/fatfs/test_fatfs_host/Makefile index 94d5c49ba..1e4734e5f 100644 --- a/components/fatfs/test_fatfs_host/Makefile +++ b/components/fatfs/test_fatfs_host/Makefile @@ -72,7 +72,7 @@ $(COMPONENT_LIB): $(OBJ_FILES) lib: $(COMPONENT_LIB) partition_table.bin: partition_table.csv - python ../../partition_table/gen_esp32part.py --verify $< $@ + python ../../partition_table/gen_esp32part.py $< $@ $(TEST_PROGRAM): lib $(TEST_OBJ_FILES) $(SPI_FLASH_SIM_DIR)/$(SPI_FLASH_LIB) $(WEAR_LEVELLING_HOST_DIR)/$(WEAR_LEVELLING_LIB) partition_table.bin g++ $(LDFLAGS) -o $(TEST_PROGRAM) $(TEST_OBJ_FILES) -L$(abspath .) -l:$(COMPONENT_LIB) -L$(SPI_FLASH_SIM_DIR) -l:$(SPI_FLASH_LIB) -L$(WEAR_LEVELLING_HOST_DIR) -l:$(WEAR_LEVELLING_LIB) -g -m32 diff --git a/components/partition_table/gen_esp32part.py b/components/partition_table/gen_esp32part.py index bfe20f48a..088559231 100755 --- a/components/partition_table/gen_esp32part.py +++ b/components/partition_table/gen_esp32part.py @@ -33,7 +33,32 @@ MAX_PARTITION_LENGTH = 0xC00 # 3K for partition data (96 entries) leaves 1K in MD5_PARTITION_BEGIN = b"\xEB\xEB" + b"\xFF" * 14 # The first 2 bytes are like magic numbers for MD5 sum PARTITION_TABLE_SIZE = 0x1000 # Size of partition table -__version__ = '1.1' +__version__ = '1.2' + +APP_TYPE = 0x00 +DATA_TYPE = 0x01 + +TYPES = { + "app" : APP_TYPE, + "data" : DATA_TYPE, +} + +# Keep this map in sync with esp_partition_subtype_t enum in esp_partition.h +SUBTYPES = { + APP_TYPE : { + "factory" : 0x00, + "test" : 0x20, + }, + DATA_TYPE : { + "ota" : 0x00, + "phy" : 0x01, + "nvs" : 0x02, + "coredump" : 0x03, + "esphttpd" : 0x80, + "fat" : 0x81, + "spiffs" : 0x82, + }, +} quiet = False md5sum = True @@ -46,9 +71,8 @@ def status(msg): def critical(msg): """ Print critical message to stderr """ - if not quiet: - sys.stderr.write(msg) - sys.stderr.write('\n') + sys.stderr.write(msg) + sys.stderr.write('\n') class PartitionTable(list): def __init__(self): @@ -85,7 +109,7 @@ class PartitionTable(list): critical("WARNING: 0x%x address in the partition table is below 0x%x" % (e.offset, last_end)) e.offset = None if e.offset is None: - pad_to = 0x10000 if e.type == PartitionDefinition.APP_TYPE else 4 + pad_to = 0x10000 if e.type == APP_TYPE else 4 if last_end % pad_to != 0: last_end += pad_to - (last_end % pad_to) e.offset = last_end @@ -109,8 +133,6 @@ class PartitionTable(list): def find_by_type(self, ptype, subtype): """ Return a partition by type & subtype, returns None if not found """ - TYPES = PartitionDefinition.TYPES - SUBTYPES = PartitionDefinition.SUBTYPES # convert ptype & subtypes names (if supplied this way) to integer values try: ptype = TYPES[ptype] @@ -197,30 +219,6 @@ class PartitionTable(list): return "\n".join(rows) + "\n" class PartitionDefinition(object): - APP_TYPE = 0x00 - DATA_TYPE = 0x01 - TYPES = { - "app" : APP_TYPE, - "data" : DATA_TYPE, - } - - # Keep this map in sync with esp_partition_subtype_t enum in esp_partition.h - SUBTYPES = { - APP_TYPE : { - "factory" : 0x00, - "test" : 0x20, - }, - DATA_TYPE : { - "ota" : 0x00, - "phy" : 0x01, - "nvs" : 0x02, - "coredump" : 0x03, - "esphttpd" : 0x80, - "fat" : 0x81, - "spiffs" : 0x82, - }, - } - MAGIC_BYTES = b"\xAA\x50" ALIGNMENT = { @@ -302,12 +300,12 @@ class PartitionDefinition(object): def parse_type(self, strval): if strval == "": raise InputError("Field 'type' can't be left empty.") - return parse_int(strval, self.TYPES) + return parse_int(strval, TYPES) def parse_subtype(self, strval): if strval == "": return 0 # default - return parse_int(strval, self.SUBTYPES.get(self.type, {})) + return parse_int(strval, SUBTYPES.get(self.type, {})) def parse_address(self, strval): if strval == "": @@ -327,6 +325,14 @@ class PartitionDefinition(object): if self.size is None: raise ValidationError(self, "Size field is not set") + if self.name in TYPES and TYPES.get(self.name, "") != self.type: + critical("WARNING: Partition has name '%s' which is a partition type, but does not match this partition's type (0x%x). Mistake in partition table?" % (self.name, self.type)) + all_subtype_names = [] + for names in (t.keys() for t in SUBTYPES.values()): + all_subtype_names += names + if self.name in all_subtype_names and SUBTYPES.get(self.type, {}).get(self.name, "") != self.subtype: + critical("WARNING: Partition has name '%s' which is a partition subtype, but this partition has non-matching type 0x%x and subtype 0x%x. Mistake in partition table?" % (self.name, self.type, self.subtype)) + STRUCT_FORMAT = "<2sBBLL16sL" @classmethod @@ -380,8 +386,8 @@ class PartitionDefinition(object): return ":".join(self.get_flags_list()) return ",".join([ self.name, - lookup_keyword(self.type, self.TYPES), - lookup_keyword(self.subtype, self.SUBTYPES.get(self.type, {})), + lookup_keyword(self.type, TYPES), + lookup_keyword(self.subtype, SUBTYPES.get(self.type, {})), addr_format(self.offset, False), addr_format(self.size, True), generate_text_flags()]) @@ -413,8 +419,9 @@ def main(): parser.add_argument('--flash-size', help='Optional flash size limit, checks partition table fits in flash', nargs='?', choices=[ '1MB', '2MB', '4MB', '8MB', '16MB' ]) parser.add_argument('--disable-md5sum', help='Disable md5 checksum for the partition table', default=False, action='store_true') - parser.add_argument('--verify', '-v', help='Verify partition table fields', default=True, action='store_false') - parser.add_argument('--quiet', '-q', help="Don't print status messages to stderr", action='store_true') + parser.add_argument('--no-verify', help="Don't verify partition table fields", action='store_true') + parser.add_argument('--verify', '-v', help="Verify partition table fields (deprecated, this behaviour is enabled by default and this flag does nothing.", action='store_true') + parser.add_argument('--quiet', '-q', help="Don't print non-critical status messages to stderr", action='store_true') parser.add_argument('--offset', '-o', help='Set offset partition table', default='0x8000') parser.add_argument('input', help='Path to CSV or binary file to parse.', type=argparse.FileType('rb')) @@ -436,7 +443,7 @@ def main(): status("Parsing CSV input...") table = PartitionTable.from_csv(input) - if args.verify: + if not args.no_verify: status("Verifying table...") table.verify() diff --git a/components/partition_table/test_gen_esp32part_host/gen_esp32part_tests.py b/components/partition_table/test_gen_esp32part_host/gen_esp32part_tests.py index a1d250340..4bd619fc0 100755 --- a/components/partition_table/test_gen_esp32part_host/gen_esp32part_tests.py +++ b/components/partition_table/test_gen_esp32part_host/gen_esp32part_tests.py @@ -7,6 +7,7 @@ import sys import subprocess import tempfile import os +import StringIO sys.path.append("..") from gen_esp32part import * @@ -236,10 +237,10 @@ class BinaryParserTests(unittest.TestCase): t.verify() self.assertEqual(3, len(t)) - self.assertEqual(t[0].type, PartitionDefinition.APP_TYPE) + self.assertEqual(t[0].type, APP_TYPE) self.assertEqual(t[0].name, "factory") - self.assertEqual(t[1].type, PartitionDefinition.DATA_TYPE) + self.assertEqual(t[1].type, DATA_TYPE) self.assertEqual(t[1].name, "data") self.assertEqual(t[2].type, 0x10) @@ -319,16 +320,18 @@ class CommandLineTests(unittest.TestCase): f.write(LONGER_BINARY_TABLE) # run gen_esp32part.py to convert binary file to CSV - subprocess.check_call([sys.executable, "../gen_esp32part.py", - binpath, csvpath]) + output = subprocess.check_output([sys.executable, "../gen_esp32part.py", + binpath, csvpath], stderr=subprocess.STDOUT) # reopen the CSV and check the generated binary is identical + self.assertNotIn("WARNING", output) with open(csvpath, 'r') as f: from_csv = PartitionTable.from_csv(f.read()) self.assertEqual(_strip_trailing_ffs(from_csv.to_binary()), LONGER_BINARY_TABLE) # run gen_esp32part.py to conver the CSV to binary again - subprocess.check_call([sys.executable, "../gen_esp32part.py", - csvpath, binpath]) + output = subprocess.check_output([sys.executable, "../gen_esp32part.py", + csvpath, binpath], stderr=subprocess.STDOUT) + self.assertNotIn("WARNING", output) # assert that file reads back as identical with open(binpath, 'rb') as f: binary_readback = f.read() @@ -356,6 +359,24 @@ app,app, factory, 32K, 1M t.verify() + def test_warnings(self): + try: + sys.stderr = StringIO.StringIO() # capture stderr + + csv_1 = "app, 1, 2, 32K, 1M\n" + PartitionTable.from_csv(csv_1).verify() + self.assertIn("WARNING", sys.stderr.getvalue()) + self.assertIn("partition type", sys.stderr.getvalue()) + + sys.stderr = StringIO.StringIO() + csv_2 = "ota_0, app, ota_1, , 1M\n" + PartitionTable.from_csv(csv_2).verify() + self.assertIn("WARNING", sys.stderr.getvalue()) + self.assertIn("partition subtype", sys.stderr.getvalue()) + + finally: + sys.stderr = sys.__stderr__ + class PartToolTests(unittest.TestCase): def _run_parttool(self, csvcontents, args): @@ -363,7 +384,11 @@ class PartToolTests(unittest.TestCase): with open(csvpath, "w") as f: f.write(csvcontents) try: - return subprocess.check_output([sys.executable, "../parttool.py"] + args.split(" ") + [ csvpath ]).strip() + output = subprocess.check_output([sys.executable, "../parttool.py"] + args.split(" ") + [ csvpath ], + stderr=subprocess.STDOUT) + self.assertNotIn("WARNING", output) + m = re.search("0x[0-9a-fA-F]+", output) + return m.group(0) if m else "" finally: os.remove(csvpath) diff --git a/components/spiffs/test_spiffs_host/Makefile b/components/spiffs/test_spiffs_host/Makefile index db94ab96f..173bb4c83 100644 --- a/components/spiffs/test_spiffs_host/Makefile +++ b/components/spiffs/test_spiffs_host/Makefile @@ -67,7 +67,7 @@ $(COMPONENT_LIB): $(OBJ_FILES) lib: $(COMPONENT_LIB) partitions_table.bin: partitions_table.csv - python ../../partition_table/gen_esp32part.py --verify $< $@ + python ../../partition_table/gen_esp32part.py $< $@ $(TEST_PROGRAM): lib $(TEST_OBJ_FILES) $(SPI_FLASH_SIM_DIR)/$(SPI_FLASH_LIB) partitions_table.bin g++ $(LDFLAGS) -o $(TEST_PROGRAM) $(TEST_OBJ_FILES) -L$(abspath .) -l:$(COMPONENT_LIB) -L$(SPI_FLASH_SIM_DIR) -l:$(SPI_FLASH_LIB) -g -m32 diff --git a/components/wear_levelling/test_wl_host/Makefile b/components/wear_levelling/test_wl_host/Makefile index 99de29942..c7f835389 100644 --- a/components/wear_levelling/test_wl_host/Makefile +++ b/components/wear_levelling/test_wl_host/Makefile @@ -66,7 +66,7 @@ force: lib: $(COMPONENT_LIB) partition_table.bin: partition_table.csv - python ../../../components/partition_table/gen_esp32part.py --verify $< $@ + python ../../../components/partition_table/gen_esp32part.py $< $@ $(TEST_PROGRAM): lib $(TEST_OBJ_FILES) $(SPI_FLASH_SIM_DIR)/$(SPI_FLASH_LIB) partition_table.bin g++ $(LDFLAGS) -o $@ $(TEST_OBJ_FILES) -L$(abspath .) -l:$(COMPONENT_LIB) -L$(SPI_FLASH_SIM_DIR) -l:$(SPI_FLASH_LIB) -g -m32 diff --git a/docs/en/api-guides/partition-tables.rst b/docs/en/api-guides/partition-tables.rst index 6363f58f8..14e5bc984 100644 --- a/docs/en/api-guides/partition-tables.rst +++ b/docs/en/api-guides/partition-tables.rst @@ -136,18 +136,16 @@ If you configure the partition table CSV name in ``make menuconfig`` and then `` To convert CSV to Binary manually:: - python gen_esp32part.py --verify input_partitions.csv binary_partitions.bin + python gen_esp32part.py input_partitions.csv binary_partitions.bin To convert binary format back to CSV:: - python gen_esp32part.py --verify binary_partitions.bin input_partitions.csv + python gen_esp32part.py binary_partitions.bin input_partitions.csv To display the contents of a binary partition table on stdout (this is how the summaries displayed when running `make partition_table` are generated:: python gen_esp32part.py binary_partitions.bin -``gen_esp32part.py`` takes one optional argument, ``--verify``, which will also verify the partition table during conversion (checking for overlapping partitions, unaligned partitions, etc.) - MD5 checksum ~~~~~~~~~~~~