optional parameters avoid append comma

[HerrMuellerluedenscheid]

Hey,

I started a driver for the sim7020 NBIoT modem using at-commands. Super helpful! Unfortunately, the handling of optional parameters seems to be different in this chip. Instead of providing empty comma separated parameters the chip seems to also make the delimiting comma optional. e.g. creating a http session (section 8.2.1 in the AT reference pdf:

        at_commands::builder::CommandBuilder::create_set(buffer, true)
            .named("+CHTTPCREATE")
            .with_string_parameter("http://blabla")
            .with_optional_string_parameter(None)
            .with_optional_string_parameter(None)
            .finish()

// should produce: AT+CHTTPCREATE=http://blabla,,

The chip will return an error. Same happened with all other commands on that IC using optional parameters. If, instead, I use:

        at_commands::builder::CommandBuilder::create_set(buffer, true)
            .named("+CHTTPCREATE")
            .with_string_parameter("http://blabla")
            .finish()

// should produce: AT+CHTTPCREATE=http://blabla

it works fine.

Is this a different flavor of AT commands in general? If not, maybe a feature flag e.g. add_comma_on_optional_commands or alike to switch the delimiters on/off? Looking forward hearing your thoughts on that. Cheers

Marius

[diondokter]

Hi there!

I see your problem. Yeah that's a bit annoying.

Adding a feature is a bad idea, because say you've got two drivers using this crate it'd force both of them to have the same behavior.

I think it'd be better to add two new functions that exhibits the behavior you want. If None, it would do nothing, if Some, it would add the parameter like normal. Should be simple to implement. The hardest part is naming the things.

The current optional parameters always leave behind a comma to signify their absence. So what would you call one that doesn't leave behind the comma? with_skipped_int_parameter?

I don't know... I can't come up with anything good. I'm open to making a breaking change to make it have good names!

If you want, you can make a PR and I'll review, merge and release it.

[diondokter]

I got a suggestion from someone to add an option to remove all trailing comma's from a command. I think that'd help in your case.

However, what also may work is splitting the builder so it's not a single line:

let mut builder = at_commands::builder::CommandBuilder::create_set(buffer, true)
    .named("+CHTTPCREATE")
    .with_string_parameter("http://blabla");

if let Some(foo) = foo {
    builder = builder.with_int_parameter(foo);
}

builder.finish()

[HerrMuellerluedenscheid]

Thanks for the proposed solution to split the builder. I guess I will use this for the time being.

How about a method .trim() to remove trailing commas. .with_skipped_int(...) would also be fine but I find the name confusing. How about .with_optionally_skipped_int(...)?

[diondokter]

I think I can get behind .with_skipped_int_parameter(...).

[HerrMuellerluedenscheid]

I will send a merge request in the coming days