Deploy contents.xml to enable automatic image flashing in Axiom #29
Conversation
vkraleti
requested review from
lumag,
ndechesne and
sbanerjee-quic
as code owners
vkraleti
requested review from
lool,
lumag,
ndechesne and
ricardosalveti
and removed request for
lumag,
ndechesne and
sbanerjee-quic
vkraleti
force-pushed
the
content-xml
branch
2 times, most recently
from
0dd4638 to
eafa663
Compare
|
@lumag @lool @ricardosalveti Based on your inputs and feedback from the test teams, I refined the contents template and rewrote gen_contents.py to generate contents.xml based on partitions.xml. There are no hard-coded build IDs anymore. Please check once. |
There was a problem hiding this comment.
Oh wow, this is sooooo much better than the previous approach, thanks for writing this!
I've left some review comments on individual commits, on top of these:
- the contents.xml.in seem almost identical between qcs9100 and qcs6490; it seems the chipset name and the presence of a SAIL flashing stage are the only differences; could we:
- have a single template with an optional SAIL section
- read the chipset name from a trivial metadata file
-
intuitively, the overall approach seems too verbose: many python lines to generate XML elements one by one; I'd have expected a much smaller lift from partitions.conf or partitions.xml to contents.xml; I wonder if this would have been simpler with simple templating like jinja2? Just an idea to make this simpler to read and hence maintain. :)
-
I ran flake8 and pycodestyle and they found a few errors and a number of stylistic improvements; would you kindly review them?
flake8 output
lminier@LMINIER-mac qcom-ptool % flake8 gen_contents.py
gen_contents.py:9:1: F401 'xml.etree.ElementTree.Element' imported but unused
gen_contents.py:9:1: F401 'xml.etree.ElementTree.SubElement' imported but unused
gen_contents.py:11:1: E302 expected 2 blank lines, found 1
gen_contents.py:12:80: E501 line too long (112 > 79 characters)
gen_contents.py:12:98: E225 missing whitespace around operator
gen_contents.py:15:1: E302 expected 2 blank lines, found 1
gen_contents.py:27:1: E302 expected 2 blank lines, found 1
gen_contents.py:68:51: E231 missing whitespace after ','
gen_contents.py:72:80: E501 line too long (81 > 79 characters)
gen_contents.py:78:80: E501 line too long (83 > 79 characters)
gen_contents.py:86:51: E231 missing whitespace after ','
gen_contents.py:90:80: E501 line too long (83 > 79 characters)
gen_contents.py:99:51: E231 missing whitespace after ','
gen_contents.py:100:80: E501 line too long (87 > 79 characters)
gen_contents.py:101:80: E501 line too long (80 > 79 characters)
gen_contents.py:102:80: E501 line too long (84 > 79 characters)
gen_contents.py:104:80: E501 line too long (84 > 79 characters)
gen_contents.py:142:80: E501 line too long (83 > 79 characters)
gen_contents.py:145:80: E501 line too long (80 > 79 characters)
gen_contents.py:147:80: E501 line too long (80 > 79 characters)
gen_contents.py:154:1: E305 expected 2 blank lines after class or function definition, found 1
gen_contents.py:155:4: E111 indentation is not a multiple of 4
gen_contents.py:164:25: E225 missing whitespace around operator
gen_contents.py:166:30: E225 missing whitespace around operator
gen_contents.py:168:27: E225 missing whitespace around operator
gen_contents.py:175:11: F541 f-string is missing placeholders
gen_contents.py:178:11: F541 f-string is missing placeholders
pycodestyle output
gen_contents.py:11:1: E302 expected 2 blank lines, found 1
gen_contents.py:12:80: E501 line too long (112 > 79 characters)
gen_contents.py:12:98: E225 missing whitespace around operator
gen_contents.py:15:1: E302 expected 2 blank lines, found 1
gen_contents.py:27:1: E302 expected 2 blank lines, found 1
gen_contents.py:68:51: E231 missing whitespace after ','
gen_contents.py:72:80: E501 line too long (81 > 79 characters)
gen_contents.py:78:80: E501 line too long (83 > 79 characters)
gen_contents.py:86:51: E231 missing whitespace after ','
gen_contents.py:90:80: E501 line too long (83 > 79 characters)
gen_contents.py:99:51: E231 missing whitespace after ','
gen_contents.py:100:80: E501 line too long (87 > 79 characters)
gen_contents.py:101:80: E501 line too long (80 > 79 characters)
gen_contents.py:102:80: E501 line too long (84 > 79 characters)
gen_contents.py:104:80: E501 line too long (84 > 79 characters)
gen_contents.py:142:80: E501 line too long (83 > 79 characters)
gen_contents.py:145:80: E501 line too long (80 > 79 characters)
gen_contents.py:147:80: E501 line too long (80 > 79 characters)
gen_contents.py:154:1: E305 expected 2 blank lines after class or function definition, found 1
gen_contents.py:155:4: E111 indentation is not a multiple of 4
gen_contents.py:164:25: E225 missing whitespace around operator
gen_contents.py:166:30: E225 missing whitespace around operator
gen_contents.py:168:27: E225 missing whitespace around operator
|
Here are the generated contents.xml files from your branch for other reviewers to see how nice they now are: |
|
@lool |
vkraleti
force-pushed
the
content-xml
branch
from
eafa663 to
4e025ae
Compare
|
Second patch, commit message, s/parition.xml/partition.xml/ |
To improve build hygiene and to ensure partitions.xml is retained,
- Add paritions.xml to the list of build targets to ensure
the generated files are not automatically deleted.
- Add 'clean' target to remove generated XML and binary files
on demand.
Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
gen_contents.py accepts a contents template and partition.xml as inputs and generates 'contents.xml' by updating image information in a format compatible with Axiom for automatic image flashing. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
Add a template which can be passed to gen_contents.py script, to generate contents.xml suitable for QCS6490-RB3GEN2 boards. The 'contents.xml' file maps images to their respective partitions in a format understood by Axiom, which is helpful to trigger automatic image flashing with Axiom. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
Add a template which can be passed to gen_contents.py script, to generate contents.xml suitable for QCS9100-RIDE boards. The 'contents.xml' file maps images to their respective partitions in a format understood by Axiom, which is helpful to trigger automatic image flashing with Axiom. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
Add a template which can be passed to gen_contents.py script, to generate contents.xml suitable for QCS8300-RIDE boards. The 'contents.xml' file maps images to their respective partitions in a format understood by Axiom, which is helpful to trigger automatic image flashing with Axiom. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
vkraleti
force-pushed
the
content-xml
branch
from
4e025ae to
71bc62d
Compare
For platforms with a contents.xml.in avilable, generate contents.xml from it using gen_contents.py Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
vkraleti
force-pushed
the
content-xml
branch
from
71bc62d to
cc55107
Compare
There was a problem hiding this comment.
Thanks, looks good to me; I saw you added the targeted pycodestyle call, and the contents.xml.in dependency. I ran make lint and make integration tests successfully.
It would be nice to merge the contents.xml.in in a single template.
(I don't know how to test the resulting contents.xml files.)
@vkraleti @quic-viskuma have worked with @smuppand and team to test them |
Created #31 to track single template ask. |
7 participants
The 'contents.xml' file maps images to their respective partitions in a format understood by Axiom. Deploy the same for supported platforms to enable Axiom automatic image flashing.