blackbirdhq / atat Goto Github PK

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.

Since it returns a stack-allocated heapless::String and not a &str, it should probably be called as_string. Otherwise, if you actually wanted a &str, you'd have to call cmd.as_str().as_str() which is a bit weird 🙂

An alternative would be as_text() or text(), not sure if you'd like that better. I think I'd prefer as_string().

Using ufmt would allow the crate to take up a much smaller memory footprint, while being much faster.

When deriving AtatEnum for an enum, i think it would make sense to be able to mark the default value (factory-programmed value) as:

/// GPIO input value (for input function <gpio_mode>=1 only):
#[derive(Clone, PartialEq, AtatEnum)]
pub enum GpioInPull {
    /// (default value): no resistor activated
    #[at_arg(default, value = 0, position = 0)
    NoPull,
    /// pull up resistor active
    #[at_arg(value = 1, position = 1)
    PullUp,
    /// pull down resistor active
    #[at_arg(value = 2, position = 2)
    PullDown,
}

Expanding the default part of #[at_arg(default, value = 0, position = 0) to:

impl Default for GpioInPull {
    fn default() -> Self {
        GpioInPull::NoPull
    }
}

In general the data buffers should be of type Vec<u8> instead of String, to support sending and receiving non-ascii data.

Currently the timeout and abortable fields are always set to None (not implemented. These should be correctly parse from the at_cmd attribute

ref:
https://github.com/BlackbirdHQ/atat/blob/5d5ba6915cee219c22e2b189fe98624f4eba50a0/atat_derive/src/lib.rs#L343-L344

I am currently writing a similar crate to interface with AT modules. I see that you have actually got something talking to a module which is far more progressed than I am.

Where my implementation differs however is that I am using traits for both creating and parsing the commands as well as sending and receiving data.

My main question is if you had any thoughts on how you want to generate the AT commands? I have created a macro that will expand an enum out into a send command, state and received data enums. These are defined by:

#[atat("\n")]
        enum TelitCommand {
            AT (None,(u8,u16), ()),
            AT_ (None,(u8,u16), (i64)),
            AT__ (None,(&'b[u8]), (u32)),
        }

This expands out to being able to send AT=u8,u16 and returns an OK(()), AT?u8,u16 and returns an i64 and AT?=[u8] and returns a u32.

The state at the moment is that the crate can create the command and send it to a generic buffer. I have started the parse side and that will take a generic buffer and return the parsed data.

I also plan to have a straight command pass through for unusual commands.

Figure out how to handle read commands where a single request will return multiple response lines followed by OK.

An example could be:

Eg.

> AT+CGACT?
< +CGACT: <cid>,<status>
< +CGACT: <cid>,<status>
...
< OK

Where +CGACT: <cid>,<status> can be repeated zero or more times, with a new cid,status pair.

Find a way to make all the genericArray items generic without ruining all ergonomics.

• ComQueue capacity
• ResQueue capacity
• UrcQueue capacity
• IngressManager buf capacity

Unit tests still need to be implemented for:

• serde_at
• atat_derive
• atat

Currently all unsolicited response codes (URCs) are being added to the response queue, which might not be the correct way of handling them, as this will force one to dequeue them before getting access to other responses.

Consider adding a handler system inspired by https://github.com/arduino-libraries/MKRGSM/blob/b13ad329093a35ee33687de90639c4561a9d6353/src/Modem.cpp#L335

Currently we rely on a timer implementing embedded_hal::Countdown for keeping track of cooldown and timeouts, but this makes use of some terrible tricks, as there is no way of obtaining the current timer value, rather than just checking if it has reached the timeout value.

Replacing the Countdown requirement with a Clock from https://github.com/FluenTech/embedded-time/.

Currently the CI will only run code tests
cargo test --tests --all-features

But it should probably be changed to run
cargo test --all-features

This requires that the all the documentation compiles, and that the examples compiles.

This would allow unit testing of small helper functions in doctests, eg.

    /// Returns `true` if `needle` is a suffix of the Vec.
    ///
    /// Always returns `true` if `needle` is an empty slice.
    ///
    /// # Examples
    ///
    /// ```
    /// use heapless::Vec;
    /// use heapless::consts::*;
    ///
    /// let mut v: Vec<_, U8> = Vec::new();
    /// v.push("a").unwrap();
    /// v.push("b").unwrap();
    ///
    /// assert!(v.ends_with(&[]));
    /// assert!(v.ends_with(&["b"]));
    /// assert!(v.ends_with(&["a", "b"]));
    /// assert!(!v.ends_with(&["a", "b", "c"]));
    /// assert!(!v.ends_with(&["b", "a"]));
    /// assert!(!v.ends_with(&["a"]));
    /// ```
    pub fn ends_with(&self, needle: &[T]) -> bool
    where
        T: PartialEq,
    {
        let (v, n) = (self.len(), needle.len());
        v >= n && needle == &self[v - n..]
    }

Currently abortable is unused.

The correct way of handling this would be to send an abort character (any character) upon command timeout, and then listen for ABORTED response code.

When connecting to an AP with the ESP8266 fails, it returns something along the lines of

WIFI DISCONNECT\r\n
WIFI CONNECTED\r\n
+CWJAP:1\r\n\r\n
FAIL\r\n

It seems that the FAIL here is used instead of ERROR. According to the reference, that 1 is an error code (timeout) and FAIL indicates the end of the response.

The AT spec (ITU V.250) does not mention such a result code. Can we handle non-standard manufacturer-specific result codes somehow?

This issue serves as a reminder of where consts generics can be applied instead of the currrent TypeNum implementation.

• AtatLen could probably be completely removed in favor of consts generics
• Queues (ComQueue, ResQueue, UrcQueue)
• BufLen

All of the above are dependent on #81

It should be possible to calculate a worst case command length given the AT command and the field types of the command struct.

ref:
https://github.com/BlackbirdHQ/atat/blob/5d5ba6915cee219c22e2b189fe98624f4eba50a0/atat_derive/src/lib.rs#L454

I will do a 0.4.0 release to crates.io as soon as

• Make buffer length and queue lengths generic (PR: #50)
• Serialize and Deserialize tuple variants (PR: #51)
• Simplify and document releasing to crates.io

is fixed and merged into master.

Currently atat_derive does a lot of manual ast parsing, that can probably be optimized, or at least made much cleaner, by using syn::parse.

Use Juniper as an example of releasing workspaces to creates: https://github.com/graphql-rust/juniper

The crate name is a bit confusing since someone is blocking the at name on crates.io. The fact that you're aliasing at_rs as at in the examples doesn't help either 🙂

What about renaming it to atat in reference to AT-AT? Star wars references already have some tradition in Rust crates. The name is short and still available on crates.io.

AT-AT could also be made into a nice logo :)

The below parameters of atatCmd could be written as eg const CAN_ABORT: bool = false;

    /// Whether or not this command can be aborted.
    fn can_abort(&self) -> bool {
        false
    }

    /// The max timeout in milliseconds.
    fn max_timeout_ms(&self) -> u32 {
        1000
    }

    /// Force the ingress manager into receive state immediately after sending
    /// the command.
    fn force_receive_state(&self) -> bool {
        false
    }

    /// Force client to look for a response.
    /// Empty slice is then passed to parse by client.
    /// Implemented to enhance expandability fo ATAT
    fn expects_response_code(&self) -> bool {
        true
    }

The current implementation assumes that ATE is enabled, in order to switch between State::Idle and State::ReceivingResponse.

With the new Digest setup, it should be fairly simple to ignore this part, and simply force it into State::ReceivingResponse every time the client calls send()

This should dramatically increase the throughput of the driver.

It should be possible to cut out all core::fmt bloat, for significant binary size wins, by switching out the log crate with defmt (https://github.com/knurling-rs/defmt)

Pros:

• Reduced binary size
• Faster logging
• Fits with defmt binaries

Cons:

• Not as flexible as log in general
• No logging on non-embedded targets

Perhaps it would be possible to make a wrapper that lets users choose between log and defmt at compile time?

Currently it is very hard to do unit tests that involve ATAT, due to the
fn send<A: AtatCmd>(&mut self, cmd: &A) -> nb::Result<A::Response, Error>; signature.

It would be nice to create and document some method of making it easy to do unittest involving sending multiple different commands after eachother, and asserting on the response.

Main problem is that even if instantiating a MockAtClient of some kind, that implements AtClient, it is very hard to construct a A::Response in a way fit for testing multiple send/receive cycles.

NOTE: This problem is not an actual problem for ATAT, but rather for the users of ATAT. Yet i think the solution should ideally come within ATAT

Add support in serde_at for handling data of the format:

/// GPIO output value (for output function <gpio_mode>=0 only):
#[derive(Clone, PartialEq, AtatEnum)]
pub enum GpioOutValue {
    #[at_arg(value = 0)]
    Low = 0,
    #[at_arg(value = 1)]
    High = 1,
}

/// GPIO input value (for input function <gpio_mode>=1 only):
#[derive(Clone, PartialEq, AtatEnum)]
pub enum GpioInPull {
    /// (default value): no resistor activated
    #[at_arg(value = 0)]
    NoPull,
    /// pull up resistor active
    #[at_arg(value = 1)]
    PullUp,
    /// pull down resistor active
    #[at_arg(value = 2)]
    PullDown,
}

#[derive(Clone, PartialEq, AtatEnum)]
pub enum GpioMode {
    #[at_arg(value = 0)]
    Output(GpioOutValue),
    #[at_arg(value = 1)]
    Input(GpioInPull),
}

More specifically serialize(GpioMode::Output(GpioOutValue::High)) in this case should result in the string 0,1

When deserializing responses in serde, it should be possible to have responses with borrowed data, eg:

pub struct SocketData<'a> {
    pub socket: u8,
    pub data: &'a str,
}

ref: https://serde.rs/lifetimes.html

Hi, is it possible that in the basic usage example

#[interrupt]
fn TIM7() {
    let ingress = unsafe { INGRESS.as_mut().unwrap() };
    ingress.parse_at();
}

it should be

    ...
    ingress.digest();
    ...

instead?

We were toying around with the idea of introducing a newtype wrapper that would allow for streaming/chunked payloads of AT commands.

Motivation

The main motivation would be to be able to reduce the stack usage of the library, by reducing the size of BufLen, as this currently needs to be able to hold the full WORST-CASE command length at all times. For all of our applications, that would be a socket ingress payload, usually in the magnitude of 512 bytes + overhead. Almost every other command we have in our applications of ATAT would be able to run with a much smaller BufLen of say 128 bytes. This could probably be even further reduced by fixing #82.

In our applications of ATAT, BufLen is 1024 bytes.

Command types to consider

• Large byte payloads (eg. socket RX)
• Vec types (eg. wifi ssid scan or cellular operator scan)

A typical command suitable for this would be AT+USORD:

Syntax:

AT+USORD=< socket >,< length >

Response:

+USORD: < socket >,< length >,< data in the ASCII [0x00,0xFF] range >
OK

Example:

AT+USORD=3,16
+USORD: 3,16,"16 bytes of data"
OK

Implementation suggestion

#[derive(Clone, AtatCmd)]
#[at_cmd("+USORD", SocketData)]
pub struct ReadSocketData {
    #[at_arg(position = 0)]
    pub socket: SocketHandle,
    #[at_arg(position = 1)]
    pub length: usize,
}

/// Example AT command with streaming payload
#[derive(Clone, AtatResp)]
pub struct SocketData<'a> {
    #[at_arg(position = 0)]
    pub socket: SocketHandle,
    #[at_arg(position = 1)]
    pub length: usize,
    #[at_arg(position = 2)]
    pub data: Streaming<'a, U128>,
}


pub fn example_using_stream() {
    match at_client.send(&ReadSocketData { socket: SocketHandle(0), length: 512 }) {
        Err(nb::Error::WouldBlock) => {
            // This would indicate that the first part of the response has yet to be received
        }
        Err(nb::Error::Other(_)) => {
            // This would indicate that an actual error occured on the AT line, 
            // or a timeout occured while waiting for the first part of the response.
        }
        Ok(response) => {
            // `response.length` can be used to verify that all chunks are received in this case
            println!("Socket: {:?}, length: {:?}", response.socket, response.length);

            // `at_client` is locked for the full lifetime of `response`, 
            // but the ingress part of `ATAT` should still be ingressing and pushing to the response queue.

            loop {
                match response.data.poll_next() {
                    Poll::Pending => {}
                    Poll::Ready(None) => break
                    Poll::Ready(chunk) => {
                        // Data can be enqueued to a socket buffer, one chunk at a time.
                        socket_layer.enqueue_slice(&chunk);
                    }
                }
            }

            // Above loop should be possible to rewrite using async, as:
            while let Some(chunk) in response.data.next().await {
                // Data can be enqueued to a socket buffer, one chunk at a time.
                socket_layer.enqueue_slice(&chunk);
            }
        }
    }
}


// Below here is given by `ATAT`
pub type ResItem<BufLen> = Result<AtItem<BufLen>, InternalError>;

pub enum AtItem<BufLen> {
    Response(Vec<u8, BufLen>),
    Chunk(Vec<u8, BufLen>)
}

pub struct Streaming<'a, BufLen> {
    consumer: &'a ResConsumer<BufLen>,
}

impl<'a, BufLen> Stream for Streaming<'a, BufLen> {
    type item = Vec<u8, BufLen>;

    fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
        match self.consumer.peek() {
            Some(Ok(AtItem::Chunk(bytes)) => {
                unsafe { self.consumer.dequeue_unchecked(); }
                Poll::Ready(Some(bytes))
            }
            // The next item in the queue is a new response
            Some(Ok(AtItem::Response(_)) => {
                Poll::Ready(None)
            }
            // FIXME: Is this correct?
            Some(Err(e)) => Poll::Ready(None),
            None => Poll::Pending
        }
    }
}

impl<'a, BufLen> Drop for Streaming<'a, BufLen> {
    fn drop(&mut self) {
        // Dequeue items until there are no more `AtItem::Chunk`
        while let Some(Ok(AtItem::Chunk(_)) = self.consumer.peek() {
            self.consumer.dequeue();
        }
    }
}

Apart from above implementation steps, the parse() function of commands needs to be altered to handle AtItem::Response & constructing Streaming,
and the Digester needs to be able to handle digesting in chunks. Perhaps a new StreamingDigester would allow non-breaking changes?

Considerations

• This should preferably not incour significant overhead if not using the Streaming wrapper
• cmd.parse(..) would need to take a reference to self.res_c, which might be tricky to get right?
• The lifetime of A::Response might be tricky to get right?
• Above implementation suggestion will not be able to handle Vec<T> response streaming. See below comment on handling Vec<T>
• Not sure if this should be implemented with futures::Stream directly, or a similar self-defined trait. Unknown part would be how to handle self: Pin<&mut Self>, cx: &mut Context<'_>
• Can this be applied to URC's as well?

Limitations

• The streaming parameter has to be the last parameter of the response! I think this will always be the case anyways, but am not sure? (This could be checked for any type deriving the AtatCmd at compile time)
• The length / chunk-size of Streaming<'a, U128> (U128 here) has to be equal to BufLen, as the AtItem::Chunk is dequeued on success. Could probably be allowed to <= BufLen, if we kept a chunk index in Streaming, and only dequeued the chunk on the remainder part, but i am not sure we would win anything by it?

Further improvements

• It would be awesome if we could fill out the size_hint() function somehow, on the responses where that would be known/can be calculated. For example for the above SocketData, the size_hint would be response.length / 128.

Some devices use SPI as transfer for their AT interface, rather than UART. It should be easy enough to allow for this crate to support those devices as well.

https://static1.squarespace.com/static/561459a2e4b0b39f5cefa12e/t/5b56371470a6ad5860a01882/1532376856126/Fanstel_AT_Commands.pdf

Should be quite easy to just change the current serial_tx in builder.rs to somthing along the lines of

pub trait Transmit {
    fn write(&mut self, bytes: &[u8]) -> Result<(), Error>;
}

impl<T> Transmit for T
where
    T: embedded_hal::serial::Write<u8>
{
    fn write(&mut self, bytes: &[u8]) -> Result<(), Error> {
        for c in bytes {
            nb::block!(self.try_write(c)).map_err(|_e| Error::Write)?;
        }
        nb::block!(self.try_flush()).map_err(|_e| Error::Write)?;
    }
}

impl<T> Transmit for T
where
    T: embedded_hal::spi::FullDuplex<u8>
{
    fn write(&mut self, bytes: &[u8]) -> Result<(), Error> {
        for c in bytes {
            nb::block!(self.try_send(c)).map_err(|_e| Error::Write)?;
        }
    }
}

As an alternative to using serde_repr, an AtatEnum derive should be added, to be used as:

#[derive(AtatEnum)]
#[at_enum(u8)]
pub enum GpioMode {
    #[at_arg(value = 0)]
    Output(GpioOutValue),
    #[at_arg(value = 1)]
    Input(GpioInPull),
    #[at_arg(value = 255)]
    PadDisabled
}

This would open up for using tuple variants in enums as the above example. (tuple variants are not allowed with a discriminator. eg. Output(GpioOutValue) = 0, is not allowed)

It would be awesome to eventually add support for the 3gpp ts 27.010 multiplexer protocol described in https://www.3gpp.org/ftp/tsg_t/tsg_t/tsgt_04/docs/pdfs/TP-99119.pdf

Ublox has an implementation guide for it here: https://www.u-blox.com/en/docs/UBX-13001887

Furthermore a C implementation for reference can be found here: https://github.com/particle-iot/gsm0710muxer/tree/d7f7b75e8d48af760c4f585f222287eb9eff9422

This would probably be as a separate crate in this workspace, and might require extensive changes to the ATAT crate itself, which would be fine.

Any contributions are welcomed!

When the ESP8266 receives data from the network, it sends an URC:

+IPD,<len>[,<remote IP>,<remote port>]:<data>

(Source: Reference manual section 5.2.24)

This is how it looks in ATAT:

[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Receiving 32 bytes
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / "+IPD,185:HTTP/1.1 200 OK\r\nServ"
[2020-03-15T22:58:58Z DEBUG atat::ingress_manager] Received URC: "+IPD,185:HTTP/1.1 200 OK\r\n"
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / "Serv"
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: false, buflen: 4)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Receiving 32 bytes
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / "er: Cowboy\r\nConnection: keep-ali"
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 32)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / ""
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 0)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Receiving 32 bytes
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / "ve\r\nContent-Type: text/plain\r\nVa"
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 32)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / ""
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 0)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Receiving 32 bytes
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / "ry: Origin\r\nDate: Sun, 15 Mar 20"
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 32)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / ""
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 0)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Receiving 32 bytes
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / "20 22:58:58 GMT\r\nContent-Length:"
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 32)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / ""
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 0)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Receiving 32 bytes
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / " 13\r\nVia: 1.1 vegur\r\n\r\n8x.2xx.2x"
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 32)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / ""
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 0)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Receiving 4 bytes
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / "x.8x"
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 4)
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Digest / Idle / ""
[2020-03-15T22:58:58Z TRACE atat::ingress_manager] Clearing buffer with invalid response (incomplete: true, buflen: 0)

(My IP is censored)

First, are multi-line URCs supported?

Second, this type of length-value encoding might be a problem for ATAT which primarily expects line termination. However, it's logical that arbitrary data received over the network may not be terminated with a regular line terminator (otherwise the ESP module would need to escape certain characters). Any ideas how to solve this?

It seems like grcov only contains coverage info on serde_at and atat_derive for some weird reason.

Figure out how to get atat included in code coverage.

Import fails and I can't seem to find it in the atat source - should I create a Client manually?

Currently the ClientBuilder creates three static queues upon initialization.

This limits the usage of atat to a single instance, as multiple calls to ClientBuilder::build will result in using the same queues.

This could be solved by passing the queues into the ClientBuilder as arguments, at the cost of API ergonomics. Some of which could be improved by a simple queues! macro

I am opening this issue as an invitation to discuss the way this crate expects implementors to structure AT commands.

TLDR

Can we find a better structure for the commands/responses, that will still align with the objectives and thoughts of this crate, while making it easier to create/maintain/derive and/or will optimize better for memory/processing.
MUST be no_std and should be able to run without alloc

Current way

Just as a sum up, the current way expects commands to look something along the lines of and enum with commands as,

use core::fmt::Write;
use heapless::{ArrayLength, String, Vec};

use at::{utils, ATCommandInterface, ATRequestType, MaxCommandLen, MaxResponseLines};

#[derive(Debug, Clone, PartialEq)]
pub enum Command {
    AT,
    GetManufacturerId,
    GetModelId,
    GetFWVersion,
    GetSerialNum,
    GetId,
    SetEcho { enable: bool },
    GetEcho,
}

#[allow(dead_code)]
#[derive(Debug)]
pub enum Response {
    ManufacturerId {
        id: String<MaxCommandLen>,
    },
    ModelId {
        id: String<MaxCommandLen>,
    },
    FWVersion {
        version: String<MaxCommandLen>,
    },
    SerialNum {
        serial: String<MaxCommandLen>,
    },
    Id {
        id: String<MaxCommandLen>,
    },
    Echo {
        enable: bool,
    },
    None,
}

impl ATRequestType for Command {
    type Command = Command;

    fn try_get_cmd(self) -> Option<Self::Command> {
        Some(self)
    }

    fn get_bytes<N: ArrayLength<u8>>(&self) -> Vec<u8, N> {
        self.get_cmd().into_bytes()
    }
}

impl ATCommandInterface for Command {
    type Response = Response;

    fn get_cmd<N: ArrayLength<u8>>(&self) -> String<N> {
        let mut buffer = String::new();
        match self {
            Command::AT => String::from("AT"),
            Command::GetManufacturerId => String::from("AT+CGMI"),
            Command::GetModelId => String::from("AT+CGMM"),
            Command::GetFWVersion => String::from("AT+CGMR"),
            Command::GetSerialNum => String::from("AT+CGSN"),
            Command::GetId => String::from("ATI9"),
            Command::SetEcho { ref enable } => {
                write!(buffer, "ATE{}", *enable as u8).unwrap();
                buffer
            }
            Command::GetEcho => String::from("ATE?"),
        }
    }

    fn parse_resp(
        &self,
        response_lines: &Vec<String<MaxCommandLen>, MaxResponseLines>,
    ) -> Response {
        if response_lines.is_empty() {
            return Response::None;
        }
        let mut responses: Vec<Vec<&str, MaxResponseLines>, MaxResponseLines> =
            utils::split_parameterized_resp(response_lines);

        let response = responses.pop().unwrap();

        match *self {
            Command::AT => Response::None,
            Command::GetManufacturerId => Response::ManufacturerId {
                id: String::from(response[0]),
            },
            _ => Response::None,
        }
    }

    fn parse_unsolicited(_response_line: &str) -> Option<Response> {
        Some(Response::None)
    }
}

But this makes a couple of things very difficult:

1. It makes doing macro generation of AT commands very difficult, due to the big coupled enum.
2. It means a number of rather large match cases for get_cmd & parse_resp, and would require the same match cases in order to implement eg. max_timeout_time, can_abort etc.

Proposed way

So, because of this my suggestion would be to restructure this into something along the lines of a ATCommand trait in this crate:

pub trait ATCommand {
    type Response;

    fn cmd<N: ArrayLength<u8>>(&self) -> String<N>;

    fn can_abort(&self) -> bool {
        false
    }

    fn max_timeout_ms(&self) -> u32 {
        1000
    }

    fn response<N: ArrayLength<u8>, L: ArrayLength<String<N>>>(&self, lines: Vec<String<N>, L>) -> Self::Response;
}

This would allow implementors to use it as:

mod commands {
    use super::*;

    /// 4.12 Card identification +CCID
    /// Returns the ICCID (Integrated Circuit Card ID) of the SIM-card. ICCID is a serial number identifying the SIM.
    pub struct GetCCID;

    impl ATCommand for GetCCID {
        type Response = Option<()>;

        fn cmd<N: ArrayLength<u8>>(&self) -> String<N> {
            String::from("AT+CCID")
        }

        fn response<N: ArrayLength<u8>, L: ArrayLength<String<N>>>(
            &self,
            lines: Vec<String<N>, L>,
        ) -> Self::Response {
            None
        }
    }

    /// 5.3 Set module functionality +CFUN
    pub struct SetModuleFunctionality {
        pub fun: Functionality,
        pub rst: Option<ResetMode>,
    }

    impl ATCommand for SetModuleFunctionality {
        type Response = Option<()>;

        fn cmd<N: ArrayLength<u8>>(&self) -> String<N> {
            let mut b = String::new();
            write!(b, "AT+CFUN={}", self.fun as u8).unwrap();
            if let Some(rst) = self.rst {
                write!(b, ",{}", rst as u8).unwrap();
            }
            b
        }

        fn response<N: ArrayLength<u8>, L: ArrayLength<String<N>>>(
            &self,
            lines: Vec<String<N>, L>,
        ) -> Self::Response {
            None
        }

        fn max_timeout_ms(&self) -> u32 {
            1_000_000
        }

        fn can_abort(&self) -> bool {
            true
        }
    }
}

Making it much easier to create macros/proc_macros to implement these in a derive crate, while giving back implementors better handling of parsing responses etc.
Also it might make it easier to add more features to the commands (I have added the max_timeout_ms and the can_abort here, both with a default implementation.

Issues

The remaining issue with this, that i have a hard time working around, is the decoupling queue

static mut REQ_Q: Option<Queue<RequestType, consts::U5, u8>> = None;
static mut RES_Q: Option<Queue<Result<ResponseType, ATError>, consts::U5, u8>,
        > = None;

Ideally I would love it if we could do:

static mut REQ_Q: Option<Queue<impl ATCommand, consts::U5, u8>> = None;
static mut RES_Q: Option<Queue<Result<<impl ATCommand>::Response, ATError>, consts::U5, u8>,
        > = None;

Or something equivalent? So this is where i leave this to brighter minds than mine. Enlighten me with the options available in this awesome language!

The examples should be updated to the new structure of the repo, and the CI rule to build them re-enabled.

This is a proposal to add optional custom matchers for response codes and error codes in the same way we currently support a custom matcher for URC's.

The general structure of both would follow the UrcMatcher perfectly.

In case of the ublox cellular family, they support 3 different verbosity levels, resulting in:

• Default: ERROR\r\n
• Numeric: +CME ERROR: 5\r\n
• Verbose: +CME ERROR: "Operation not allowed"\r\n

Currently this crate only supports handling the default case, which i think makes sense, as the two others are very device specific. But a custom ErrorMatcher would open up for much better error handling in the device driver crate.

In the same way the ESP family has commands that on success returns SEND OK, as described in #32 . A custom ResponseCodeMatcher would allow handling these device specific response codes as well, without bloating the Atat crate.

Also it would move response codes "@" and ">" into a custom response code matcher.

Question is then, would it make sense to unite all 3 types of matchers under one trait:

pub enum MatcherResult<L: ArrayLength<u8>> {
    NotHandled,
    Incomplete,
    UrcComplete(String<L>),
    ResponseComplete(String<L>),
    ErrorComplete(String<L>)
}

pub trait Matcher {
    type MaxLen: ArrayLength<u8>;

    fn process(&mut self, buf: &mut String<consts::U256>) -> MatcherResult<Self::MaxLen>;
}

+CWMODE_CUR: 1 parses correctly while +CWMODE_CUR:1 fails with EofWhileParsingList.

The problem seems to be caused by https://github.com/BlackbirdHQ/atat/blob/master/serde_at/src/de/mod.rs#L482. If I remove it both versions parse correctly.

However, I noticed that for the ublox chips responses with multiple parameters are surrounded by () so eat_char is required for that. I misread that.

I'm running into problems with deserialization of Ipv4Addr structs without quotes around them.
As this is in the specs of the odin-w2 series wifi modules supported by the Ublox-short-range driver listed in the ReadMe of this project.

Specific URC in question: "+UUDPC:2,2,0,192.168.128.208,59396,172.217.19.78,80"

The digest() function of ingress manager should be exhaustive, meaning it should empty the receive buffer in a single call, instead of the current single state transition.

This would make it much easier to call digest from DMA interrupts, eg. after a character match on \r

I am not convinced the AtatClient trait brings anything to the table, that couldn't be solved using the concrete type?

Consider removing the trait entirely, in order to just make users use the concrete atat::Client type instead.

The ESP32 returns the sent request as an echo thus i cannot simply derive the AtatResp but have to manually parse the response.

Here is an example.
This is the request i send:

#[at_cmd("+CWJAP", WifiConnectResponse)]
pub struct WifiConnect {
  #[at_arg(position = 0)]
  pub ssid: String<64>,
  #[at_arg(position = 1)]
  pub pw: String<128>,
}

This is the raw response as returned from the esp32:

AT+CWJAP="ssid","password"
WIFI CONNECTED
WIFI GOT IP

OK

It would be nice if the above response could be parsed with the following AtatResp derive.

#[derive(Clone, Debug, AtatResp)]
pub struct WifiConnectResponse {
  #[at_arg(position = 0)]
  pub connected: String<32>,
  #[at_arg(position = 1)]
  pub got_ip: String<32>,
}

serde_at is based on serde_json_core, and currently still contains a few checks only related to JSON data. These can be cleaned up.

Also serde_at should probably be able to handle enums containing data better.

Generally scatter rust docs on all functions.

• atat
• atat_derive
• serde_at