Cannot parse ATI - "ParseString" error

[mon]

I'm on 0.8.4 at the moment and trying to parse the manufacturer revision information. I've tried working through serde_at/src/de/mod.rs to try and understand the failure but can't quite grok it.

The request I'm sending:

#[derive(Clone, AtatCmd)]
#[at_cmd("I", responses::Ident)]
pub struct IdentificationInformation;

The response struct:

#[derive(Clone, Debug, AtatResp, Serialize)]
pub struct Ident {
    pub revision: String<consts::U128>,
}

The raw logs:

DEBUG [atat::client:112] Sending command: ""ATI""
DEBUG [atat::ingress_manager:118] Received response ""Quectel\r\nUC20\r\nRevision: UC20GQBR03A12E1G""

However, the ParseString error is always returned. Any ideas? I get the feeling the Quectel\r\nUC20\r\n section of the string is confusing the parser, as the modem doesn't echo back the ATI first.

[MathiasKoch]

Just to get a base understanding here.

as the modem doesn't echo back the ATI first

Is this due to you not having ATE enabled, or is the ATI command a special-case, that doesn't ever echo? In case of the former, i think your issue is that we currently rely on the echo to distinguish between URC's, and command responses. This is definitely something we want to improve, and i think the road towards it was paved by #70, so it should be possible. See #82 for more on this.

In case of the latter, this might be solvable by adding force_receive_state = true to the at_cmd, like so: #[at_cmd("I", responses::Ident, force_receive_state = true)]. That will blindly force the ingress end of atat into an Receiving state after sending the command, and thus not require the echo to be present. This is also the basic suggestion to solving #83, but i haven't had time to test if it works in all cases.

If this keeps being a problem, please report back here, and i will try to setup a small test to see if i can replicate :)

[mon]

ATE is on in this case. I actually had confused my concepts a bit - commands such as AT+CGREG? which print +CGREG: 0,1 were what I was thinking of as "echo". However, commands like AT+CGSN print the bare serial number without any prefix, which is parsed correctly by atat. Because of this confusion on my part, force_receive_state = true doesn't work since it is entering the receive state with no issues.

One thing I've noticed about all the other commands we're executing is their results always are single-line. ATI is unique in that it returns 3 lines before the empty line and OK. Could these be throwing off the parser?

[MathiasKoch]

Ahh, alright. In that case i think i get the issue.

Do you have the option of testing of the full rx,tx in a regular terminal, and posting the full correspondance here? That way i can add a regression test to test it out, and at the same time make sure it doesn't break in the future.

Something along the lines of:

> ATI
< ATI
< Quectel
< UC20
< Revision: UC20GQBR03A12E1G
<
< OK

One other thing to notice, is that String expects the response to be surrounded by ", but this also does seem to be the case here, based on your raw logs?

[mon]

I think the logs just put extra quotes around things. Here's an example correspondence with some "normal" commands as well - I don't have local echo turned on, so the commands you're seeing are the ones sent back by ATE being on.

AT
OK
AT+CGREG?
+CGREG: 0,1

OK
AT+CGMI
Quectel

OK
ATI
Quectel
UC20
Revision: UC20GQBR03A12E1G

OK

[MathiasKoch]

Cool :+1:

Does CGMI parse correctly with atat? It also seems to be a charvec rather than a string

[mon]

Yes it does! The struct for that is

#[derive(Clone, Debug, AtatResp, Serialize)]
pub struct ManufacturerId {
    #[at_arg(position = 0)]
    pub id: String<consts::U64>,
}

[MathiasKoch]

Okay, cool! Then i guess the string thingy is indeed a logging thing.

I will try to setup a regression test on this, to see if i can figure out why ATI doesn't seem to parse..

What you could try in the meantime, is to attempt with the newly introduced CharVec type: https://github.com/BlackbirdHQ/atat/blob/c57de3d23e99301a10dfaa44c07427186c0df1d0/serde_at/src/de/mod.rs#L22-L119

use serde_at::CharVec;

#[derive(Clone, Debug, AtatResp)]
pub struct IdentificationInformationResponse {
    pub app_ver: CharVec<consts::U32>,
}

Not sure if that requires you to update atat version though?

I will be away most of the coming week for easter, so i might not be able to test it out until i get back.

[mon]

I'm trying to avoid updating to 0.9.0 because of the removal of log-logging. I think defmt's inablity to produce logs using stdout/logging is a problem with defmt, not atat, so once I can work out how on earth parsing of the interned strings works I'll probably open a PR upstream to provide a std-based global_logger.

Luckily, 0.8.4 still works (almost!) with CharVec - I get back a vec of bytes, which converted to a string becomes QuectelUC20Revision:UC20GQBR03A12E1G - the correct response, minus newlines.

Our use of ATI is to try and determine common factors in flaky modems - the newlines aren't important because I can pull out the revision anyway. This is a great solution for now!

[MathiasKoch]

Ahh, yeah thats fair enough (Sorry for bluntly removing the log_logging, wasn't aware of anyone using it :) I would happily take a PR that adds it again, if you were to make it?)

I think it should be possible to make a defmt -> log facade that kinda just acts as if it had been a regular log::logger. That would indeed be a great addition to the defmt infrastructure.

[MathiasKoch]

@mon Just to let you know, the log logging is now back, thx to @fkohlgrueber and #92

I would still like to investigate the ATI command & add a regression test, when i get time, so i will leave this issue open for now.

[mon]

Excellent news! I'll update to 0.10.0 next time I add more features to my project. Great to be back in line with master :)

[MathiasKoch]

It will be part of 0.11.0, so you should probably update to that one instead 😉

I won't do the release until we have the heapless dependency bump merged though, unless you strictly require it with heapless 0.5.5?

[mon]

Nope! No problems there

[MathiasKoch]

@mon Released :tada: https://crates.io/crates/atat

Give it a spin when you get time, and let me know if anything is not working.

[mon]

Been a few months but finally got time to get back into this - main issues are just updating the initialization boilerplate from 0.8.4 to 0.11.1, the examples appear to be out of date again.

Specifically, Queue(heapless::i::Queue::u8()); for all the initializers doesn't work because heapless::i doesn't contain Queue in heapless 0.7.0. I fixed it using Queue::new() instead.

My own extensions have broken too - <A as AtatCmd<LEN>>::Error` doesn't implement `std::fmt::Debug` is breaking my "coerce everything to anyhow::Error" strategy, is there better way to get the error as a string in newer atat? I have a macro that creates wrapper functions around my requests, an example as the compiler sees it is:

    pub fn get_manufacturer<A: AtatCmd<LEN>, const LEN: usize>(&mut self) -> Result<<requests::GetManufacturerId as A>::Response> {
        self.send(&requests::GetManufacturerId)
    }

0.11.1 broke this too due to cannot find associated type `Response` in `A` and I'm not sure whether there's some syntax magic to fix it, whether it's impossible or if there's a better way.

Thanks for continuing to help out with this - it seems I still have a lot of rust to learn!

[MathiasKoch]

Hi!

I am sorry about the state of the documentation & examples. They do seem to be a bit out of date. Guess i should really push to have them fixed and added to the CI.

Specifically, Queue(heapless::i::Queue::u8()); for all the initializers doesn't work because heapless::i doesn't contain Queue in heapless 0.7.0. I fixed it using Queue::new() instead.

This would be the correct way :+1:

My own extensions have broken too - <A as AtatCmd>::Errordoesn't implementstd::fmt::Debug` is breaking my "coerce everything to anyhow::Error" strategy, is there better way to get the error as a string in newer atat?

This would be because A::Error is no longer forced to implement Debug. What you could do in your case would be:

    pub fn get_manufacturer<A: AtatCmd<LEN> + Debug, const LEN: usize>(&mut self) -> Result<<requests::GetManufacturerId as A>::Response> {
        self.send(&requests::GetManufacturerId)
    }

0.11.1 broke this too due to cannot find associated type Response in A and I'm not sure whether there's some syntax magic to fix it, whether it's impossible or if there's a better way.

I think this is a follow-up error on the Debug one? A::Response should at least still be a thing..

[mon]

I might actually need another feature to complete this - want me to open a new issue or keep commenting on this one?

So this works:

    pub fn get_manufacturer(&mut self) -> Result<<requests::GetManufacturerId as AtatCmd<10>>::Response> {
        self.send(&requests::GetManufacturerId)
    }

But obviously 10 has to be entered manually. I came very close by using the LEN from AtatLen:

    pub fn get_manufacturer(&mut self) -> Result<<requests::GetManufacturerId as AtatCmd<{<requests::GetManufacturerId as AtatLen>::LEN}>>::Response> {
        self.send(&requests::GetManufacturerId)
    }

...but this is just the struct len, not the ident len as well.

Would a viable solution be to provide a CMD_LEN or similar in the impl of AtatLen? I can make the PR for that if you reckon it's a good idea.

Edit: This appears to be something that the rust compiler could eventually support with _ inferring the const parameter for AtatCmd, but is not yet ready: https://github.com/rust-lang/rust/issues/80528

[mon]

In addition, it took some work but I got the send function to work by constraining the type of Error, I wasn't aware you could restrict trait member types like this!

    fn send<A: AtatCmd<LEN, Error=atat::GenericError>, const LEN: usize>(&mut self, cmd: &A) -> Result<A::Response> {
        self.client.send(cmd).map_err(|e| anyhow!("AT error: {:?}", e))
    }

[mon]

Apologies, but I've had to give up trying to port my serial ingest to the new atat - I'm leaving my current company and won't be doing any more rust for the foreseeable future... Thank you for your time so far, it's been a pleasure.

[MathiasKoch]

Sorry to hear you didn't manage to make it work, but hey! Congratz on the new job & best of luck!