search

diondokter / at-commands

Rust project for building and parsing at commands
Apache License 2.0
33 stars 5 forks source link

Add support for optional parameters #2

Closed AlyoshaVasilieva closed 4 years ago

AlyoshaVasilieva commented 4 years ago

This PR adds support for optional parameters.

I've recently been working on interfacing with Espressif's ESP-AT firmware to add wifi to a Rust microcontroller project. ESP-AT makes fairly extensive use of optional parameters, some of which it may be important to avoid setting. e.g. I don't want to specify my wifi's MAC address, but the parameter for 'reconnect to wifi if connection lost' occurs after BSSID.

I couldn't figure out how to add support when using the current method of adding commas after the previous parameter when writing a new parameter, so instead a comma is added immediately after each parameter is written. The final comma is dropped in finish_with.

All of the previous tests pass without modification and I've added some new tests.

This does have one negative effect, though I don't think it's very realistic: if a user was using finish_with(b"") (no terminator) and a buffer of the exact size of the set command then this new method will trigger an error since we're overrunning the buffer. The only reason I could see someone doing this is to hardcode writing the terminator somewhere in order to save 1-2 bytes of memory. I don't think this is very likely. In every other situation the buffer will always have enough space for the comma if it has enough space for the terminator.

It is of course possible I missed something, as I only started working with AT commands a couple weeks ago.

diondokter commented 4 years ago

It's late here for me, so I'll review tomorrow. But from what I've skimmed through, it looks great! Thanks!

diondokter commented 4 years ago

Ok, it's looking good! Checked the flash size and that seems fine too. Good job 🙂

diondokter commented 4 years ago

@AlyoshaVasilieva FYI, I made a new release with your contribution. It also contains an experimental parser I was writing. Could be interesting depending on your use case

AlyoshaVasilieva commented 4 years ago

Some of ESP-AT's responses slightly violate proper AT design (no quotes around the HTTPCLIENT data string param) and some I think only barely qualify as AT-related (version info output is just a block of text with an OK ending). So I'll have to write a bunch of parsing specific to it. At least some of them do seem to be normal AT responses, though, so I'll parse what I can using this crate and will report any issues I run into (if I think they're issues in the parser and not Espressif doing weird things).

diondokter commented 4 years ago

Even if espressif is doing weird stuff, I'm not opposed to making it flexible enough to make it work. The current parser has an 'expect_identifier' method in it that can expect any static text. So a lot can be caught in there.

Anyways, thanks for your work so far!