Skip to content

Change XML parser#239

Merged
mathias-arm merged 2 commits intopyocd:mainfrom
mbrossard:roxmltree
Feb 24, 2025
Merged

Change XML parser#239
mathias-arm merged 2 commits intopyocd:mainfrom
mbrossard:roxmltree

Conversation

@mbrossard
Copy link
Collaborator

Fix for #236 change from minidom to roxmltree.

@mbrossard mbrossard requested a review from ithinuel February 22, 2025 22:54
@mbrossard mbrossard force-pushed the roxmltree branch 2 times, most recently from 40b745e to d762c1e Compare February 23, 2025 21:48
@mbrossard
Copy link
Collaborator Author

@Yatekii, @ithinuel: I am more concerned with the correctness than the quality of the code (although I applied the change you suggested and ran cargo clippy):

The new version yields identical results for dump-devices with 1500+ packs.

$ time ./target/release/cmsis-cli dump-devices -b minidom-boards.json -d minidom-devices.json
...
real	0m9.319s
user	0m1.556s
sys	0m7.580s

$ time ./target/release/cmsis-cli dump-devices -b roxmltree-boards.json -d roxmltree-devices.json
...
real	0m9.333s
user	0m1.563s
sys	0m7.571s

$ cmp minidom-boards.json roxmltree-boards.json ; echo $?
0
$ jq -S "." minidom-devices.json > minidom-devices.jq.json
$ jq -S "." roxmltree-devices.json > roxmltree-devices.jq.json
$ cmp minidom-devices.jq.json roxmltree-devices.jq.json ; echo $?
0

Also there seems to be no performance change.

Could you suggest some target-gen tests?

@mbrossard
Copy link
Collaborator Author

I have tested with target-gen arm with this branch and except for one minor thing it generates identical YAML files.

There are 3 files where some of the memory entries are sometimes listed in a different order. As an example SAMD51.yaml generated from Microchip.SAMD51_DFP.3.8.246.pdsc:

--- minidom/SAMD51.yaml
+++ roxmltree/SAMD51.yaml
@@ -27,7 +27,7 @@
     cores:
     - main
   - !Generic
-    name: IRAM3
+    name: IRAM2
     range:
       start: 0x20000000
       end: 0x20008000
@@ -36,7 +36,7 @@
     access:
       execute: false
   - !Generic
-    name: IRAM2
+    name: IRAM3
     range:
       start: 0x20000000
       end: 0x20008000

Those entries have the same start/end addresses. The information for those entries is passed in HashMap (keyed by the name) which explains why they can have a different order.

@mbrossard mbrossard requested a review from Yatekii February 23, 2025 22:42
@Yatekii
Copy link
Contributor

Yatekii commented Feb 23, 2025

Amazing stuff! I went through all the code yesterday and I did not see any deviations in the logic. Considering target-gen chucks away happily, I would merge this :) Thanks a lot for the work!

@mbrossard mbrossard marked this pull request as ready for review February 24, 2025 03:55
@mbrossard
Copy link
Collaborator Author

I squashed the commits and did some more clippy fixes.

@mathias-arm mathias-arm merged commit 820a9cb into pyocd:main Feb 24, 2025
12 checks passed
@mbrossard mbrossard deleted the roxmltree branch February 24, 2025 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants