Skip to content

Commit

Permalink
Fix unexpected firmware selection by flashing scripts (#12345)
Browse files Browse the repository at this point in the history
Currently the flashing scripts will flash a firmware .bin from the
current directory, if it exists with the right filename, and otherwise
finds the firmware in the same directory as the wrapper.

It is confusing that changing directories influences the firmware
selection. Downloading a built firmware archive, unpacking it, and
running the wrapper should always flash the firmware that was unpacked.

Remove the search logic. Similarly, remove the concept of detection of
"optional" firmware constituents by file presence. The command line
should determine the requisite artifacts; continuing when expected files
are missing is not desirable.

Finally, fix rebuilds of flashing scripts after the python generator's
imports change by adding those to inputs in the build file.
  • Loading branch information
mspang authored and pull[bot] committed Jan 3, 2023
1 parent 3f70770 commit 1046144
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 50 deletions.
3 changes: 3 additions & 0 deletions build/toolchain/flashable_executable.gni
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ template("gen_flashing_script") {
[
"flashing_script_generator",
"flashing_script_name",
"flashing_script_inputs",
"flashing_options",
"deps",
"data_deps",
Expand All @@ -72,6 +73,7 @@ template("gen_flashing_script") {
]

script = flashing_script_generator
inputs = flashing_script_inputs
}
}

Expand Down Expand Up @@ -131,6 +133,7 @@ template("flashable_executable") {

gen_flashing_script("$target_name.flashing") {
flashing_script_generator = invoker.flashing_script_generator
flashing_script_inputs = invoker.flashing_script_inputs
flashing_script_name = "$root_out_dir/${invoker.flashing_script_name}"
if (defined(invoker.flashing_options)) {
flashing_options = invoker.flashing_options
Expand Down
4 changes: 2 additions & 2 deletions scripts/flashing/efr32_firmware_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ def actions(self):
if self.erase().err:
return self

application = self.optional_file(self.option.application)
if application:
if self.option.application:
application = self.option.application
if self.flash(application).err:
return self
if self.option.verify_application:
Expand Down
15 changes: 9 additions & 6 deletions scripts/flashing/esp32_firmware_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"""

import os
import pathlib
import sys

import firmware_utils
Expand Down Expand Up @@ -222,7 +223,8 @@
'help': 'Bootloader image',
'default': None,
'argparse': {
'metavar': 'FILE'
'metavar': 'FILE',
'type': pathlib.Path,
},
},
'bootloader_offset': {
Expand All @@ -237,7 +239,8 @@
'help': 'Partition table image',
'default': None,
'argparse': {
'metavar': 'FILE'
'metavar': 'FILE',
'type': pathlib.Path,
},
},
'partition_offset': {
Expand Down Expand Up @@ -408,11 +411,11 @@ def actions(self):
if self.erase().err:
return self

bootloader = self.optional_file(self.option.bootloader)
application = self.optional_file(self.option.application)
partition = self.optional_file(self.option.partition)
if self.option.application:
application = self.option.application
bootloader = self.option.bootloader
partition = self.option.partition

if bootloader or application or partition:
# Collect the flashable items.
flash = []
if bootloader:
Expand Down
35 changes: 11 additions & 24 deletions scripts/flashing/firmware_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"""Utitilies to flash or erase a device."""

import argparse
import pathlib
import errno
import locale
import os
Expand Down Expand Up @@ -62,7 +63,8 @@
'help': 'Flash an image',
'default': None,
'argparse': {
'metavar': 'FILE'
'metavar': 'FILE',
'type': pathlib.Path,
},
},
'verify_application': {
Expand Down Expand Up @@ -320,8 +322,8 @@ def format_command(self, template, args=None, opt=None):
↦ ρᵢ if opt[name]==σᵢ
ρ otherwise
"""
if isinstance(template, str):
result = [template.format_map(opt)]
if isinstance(template, str) or isinstance(template, pathlib.Path):
result = [str(template).format_map(opt)]
elif isinstance(template, list):
result = []
for i in template:
Expand Down Expand Up @@ -362,26 +364,6 @@ def format_command(self, template, args=None, opt=None):
raise ValueError('Unknown: {}'.format(template))
return result

def find_file(self, filename, dirs=None):
"""Resolve a file name; also checks the script directory."""
if os.path.isabs(filename) or os.path.exists(filename):
return filename
dirs = dirs or []
if self.argv0:
dirs.append(os.path.dirname(self.argv0))
for directory in dirs:
name = os.path.join(directory, filename)
if os.path.exists(name):
return name
raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT),
filename)

def optional_file(self, filename, dirs=None):
"""Resolve a file name, if present."""
if filename is None:
return None
return self.find_file(filename, dirs)

def parse_argv(self, argv):
"""Handle command line options."""
self.argv0 = argv[0]
Expand Down Expand Up @@ -426,10 +408,15 @@ def make_wrapper(self, argv):
defaults = []
for key, value in vars(args).items():
if key in self.option and value != getattr(self.option, key):
defaults.append(' {}: {},'.format(repr(key), repr(value)))
if isinstance(value, pathlib.Path):
defaults.append(' {}: os.path.join(os.path.dirname(sys.argv[0]), {}),'.format(
repr(key), repr(str(value))))
else:
defaults.append(' {}: {},'.format(repr(key), repr(value)))

script = """
import sys
import os.path
DEFAULTS = {{
{defaults}
Expand Down
4 changes: 2 additions & 2 deletions scripts/flashing/nrfconnect_firmware_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ def actions(self):
if self.erase().err:
return self

application = self.optional_file(self.option.application)
if application:
if self.option.application:
application = self.option.application
if self.flash(application).err:
return self
if self.option.verify_application:
Expand Down
4 changes: 2 additions & 2 deletions scripts/flashing/p6_firmware_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ def actions(self):
if self.erase().err:
return self

application = self.optional_file(self.option.application)
if application:
if self.option.application:
application = self.option.application
if self.flash(application).err:
return self
if self.option.verify_application:
Expand Down
4 changes: 2 additions & 2 deletions scripts/flashing/qpg_firmware_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ def actions(self):
if self.erase().err:
return self

application = self.optional_file(self.option.application)
if application:
if self.option.application:
application = self.option.application
if self.flash(application).err:
return self
if self.option.verify_application:
Expand Down
9 changes: 5 additions & 4 deletions third_party/efr32_sdk/efr32_executable.gni
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ template("efr32_executable") {
# or in different containers.

flashing_runtime_target = target_name + ".flashing_runtime"
flashing_script_inputs = [
"${chip_root}/scripts/flashing/efr32_firmware_utils.py",
"${chip_root}/scripts/flashing/firmware_utils.py",
]
copy(flashing_runtime_target) {
sources = [
"${chip_root}/scripts/flashing/efr32_firmware_utils.py",
"${chip_root}/scripts/flashing/firmware_utils.py",
]
sources = flashing_script_inputs
outputs = [ "${root_out_dir}/{{source_file_part}}" ]
}

Expand Down
10 changes: 6 additions & 4 deletions third_party/p6/p6_executable.gni
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ template("p6_executable") {
#or in different containers.

flashing_runtime_target = target_name + ".flashing_runtime"
flashing_script_inputs = [
"${chip_root}/scripts/flashing/firmware_utils.py",
"${chip_root}/scripts/flashing/p6_firmware_utils.py",
]

copy(flashing_runtime_target) {
sources = [
"${chip_root}/scripts/flashing/firmware_utils.py",
"${chip_root}/scripts/flashing/p6_firmware_utils.py",
]
sources = flashing_script_inputs
outputs = [ "${root_out_dir}/{{source_file_part}}" ]
}

Expand Down
10 changes: 6 additions & 4 deletions third_party/qpg_sdk/qpg_executable.gni
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ template("qpg_executable") {
# or in different containers.

flashing_runtime_target = target_name + ".flashing_runtime"
flashing_script_inputs = [
"${chip_root}/scripts/flashing/firmware_utils.py",
"${chip_root}/scripts/flashing/qpg_firmware_utils.py",
]

copy(flashing_runtime_target) {
sources = [
"${chip_root}/scripts/flashing/firmware_utils.py",
"${chip_root}/scripts/flashing/qpg_firmware_utils.py",
]
sources = flashing_script_inputs
outputs = [ "${root_out_dir}/{{source_file_part}}" ]
}

Expand Down

0 comments on commit 1046144

Please sign in to comment.