Skip to content

Fix parsing xml from firehose#14

Merged
quic-kdybcio merged 1 commit intoqualcomm:mainfrom
gfabiano:firehose_multi_xml
Jul 7, 2025
Merged

Fix parsing xml from firehose#14
quic-kdybcio merged 1 commit intoqualcomm:mainfrom
gfabiano:firehose_multi_xml

Conversation

@gfabiano
Copy link
Copy Markdown
Contributor

@gfabiano gfabiano commented Jul 2, 2025

Fix parsing xml from firehose when multiple xml are sent sequentially on the same data transfer.

Loader sent. Hack away!
Firehose failed. Resetting the board to edl, try again.
RECV: Element { prefix: None, namespace: None, namespaces: None, name: "response", attributes: {"value": "ACK", "rawmode": "false"}, children: [] }
Error: Malformed XML. 4:17 Invalid processing instruction: <?xml - "<?xml"-like PI is only valid at the beginning of the document

@gfabiano gfabiano force-pushed the firehose_multi_xml branch 2 times, most recently from daae138 to 5d979ed Compare July 2, 2025 14:10
@quic-kdybcio
Copy link
Copy Markdown
Contributor

Huh, interesting.. the spec doesn't really enforce there's only a single XML per transfer, but this hasn't been an issue on devices I've tested so far.

Could you share some details about your setup?

@gfabiano
Copy link
Copy Markdown
Contributor Author

gfabiano commented Jul 3, 2025

Hi @quic-kdybcio !

Yes my board is based on sa8155p, when I try to flash with this tool I noticed that it failed and I investigated, what i noticed is that I received multiple xml on one transfer. I do not know the spec of firehose and I'm not so expert on that. Maybe is an edge case. If you need some specific test I can try.

QDL and QFIL never failed by the way so i think that they handle that.

@quic-kdybcio
Copy link
Copy Markdown
Contributor

@gfabiano I came up with this diff to reduce copying and loosen the encoding/whitespace requirements, I only compile-tested it - can you give it a shot?

index b15029b89f04..16960e8e8930 100644
--- a/qdl/src/lib.rs
+++ b/qdl/src/lib.rs
@@ -9,6 +9,7 @@ use serial::setup_serial_device;
 use std::cmp::min;
 use std::io::Read;
 use std::io::Write;
+use std::str;
 use std::str::FromStr;
 use types::FirehoseResetMode;
 use types::FirehoseStatus;
@@ -96,15 +97,13 @@ pub fn firehose_read<T: Read + Write + QdlChan>(

         got_any_data = true;

-        let xml_declaration = r#"<?xml version="1.0" encoding="UTF-8" ?>"#;
         let xml_fragments: Vec<&str> = str::from_utf8(&buf[..bytes_read])?
-            .split(xml_declaration)
+            .split_inclusive("<?xml")
             .filter(|s| !s.is_empty())
             .collect();

         for fragment in xml_fragments.iter() {
-            let full_xml_string = format!("{}{}", xml_declaration, fragment);
-            let xml = xmltree::Element::parse(full_xml_string.as_bytes())?;
+            let xml = xmltree::Element::parse(fragment.as_bytes())?;

             if xml.name != "data" {
                 // TODO: define a more verbose level
@@ -113,7 +112,7 @@ pub fn firehose_read<T: Read + Write + QdlChan>(
                 }
                 bail!("Got a firehose packet without a data tag");
             }
-
+
             // The spec expects there's always a single node only
             if let Some(XMLNode::Element(e)) = xml.children.first() {
                 // Check for a 'log' node and print out the message

@gfabiano
Copy link
Copy Markdown
Contributor Author

gfabiano commented Jul 3, 2025

Yes for sure, I fetch and try to flash something

@gfabiano
Copy link
Copy Markdown
Contributor Author

gfabiano commented Jul 3, 2025

Hi @quic-kdybcio,

split_inclusive unfortunately add the delimiter not in the right place. So I modified the implementation to use indexes and slices to better optimize and avoid unintended copies. I flashed without problems on my board.

Give me some feedback if it is ok or it needs reworks.

@gfabiano
Copy link
Copy Markdown
Contributor Author

gfabiano commented Jul 4, 2025

I solved the feedback as requested

… on the same data transfer

Signed-off-by: Giuseppe Fabiano <gfabiano40@gmail.com>
@quic-kdybcio
Copy link
Copy Markdown
Contributor

Squashed your commits and resolved a cosmetic merge conflit - now just waiting for CI to run

@quic-kdybcio quic-kdybcio merged commit e88d648 into qualcomm:main Jul 7, 2025
6 checks passed
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.

2 participants