Consistent default serial baud

[bogdanm]

This commit changes the default baud of a new Serial or SerialBase
instance to the one defined by the core configuration
(MBED_CONF_CORE_STDIO_BAUD_RATE). This should ensure a consistent
behaviour of newly created Serial objects across all platforms.

Related to #2396.

[bogdanm]

/morph test

[bogdanm]

Before this PR, the default baud rate of a serial object was defined in its HAL implementation. The PR might break some existing code, but I personally don't think it's worse than (say) this:

$ ack-grep -w STDIO_BAUD .
TARGET_NXP/TARGET_LPC43XX/TARGET_LPC4337/PeripheralNames.h
92:#define STDIO_BAUD        9600

TARGET_NXP/TARGET_LPC43XX/TARGET_LPC4330/PeripheralNames.h
92:#define STDIO_BAUD        115200

TARGET_NXP/TARGET_LPC43XX/serial_api.c
166:        serial_baud (obj, STDIO_BAUD);

I think we should bite the bullet and go ahead with it.
@0xc0170 @sg-

[bogdanm]

Interesting. This seems to have failed because somehow build.py doesn't get along with the configuration system. I'll investigate. Please don't merge for now (but feel free to comment anyway).

[0xc0170]

What's the default value for MBED_CONF_CORE_STDIO_BAUD_RATE ?

[bogdanm]

Whatever you want it to be! :) Right now it's 9600:

https://github.com/ARMmbed/mbed-os/blob/master/mbed_lib.json#L11

[geky]

Sorry I'm late to the party.

Are we sure we want to bind stdio config and the Serial class together like this? It seems like these are two independent concepts (stdio could be over spi for all the application cares).

As a user, I would find it surprising if I changed my terminal baud-rate and suddenly the uart underlying a homemade driver started failing.

[bridadan]

@geky That's a really good point. I'm inclined to agree on that. In the past we've only slightly differentiated between stdio and serial. But maybe we need to start clearly documenting where these two entities differ?

[bogdanm]

As a user, I would find it surprising if I changed my terminal baud-rate and suddenly the uart underlying a homemade driver started failing.

I get what you're saying, the problem is that we allow people to instantiate Serial objects without specifying a baud rate (which is wrong to begin with IMO), and by doing that they end up relying on the platform's HAL implementation to choose a baud rate. That might not be what they want; even if it is, the HAL's baud rate might change unexpectedly. #2396 makes a good case for this PR, I think. Plus, there's quite a bit of tradition in mbed to associate the terminal baud rate with your serial instance baud rate, simply because until recently the only good way to change the default terminal baud rate was to instantiate a Serial object on top of it and call baud on that.
I'm not saying this PR fixes everything, but it feels less wrong and maybe a bit more consistent. I'm of course open to better solutions.

[bridadan]

I think in this case, exposing the Serial class's default baud rate as a config option would be confusing. The last thing I want to do is start adding *s to all of our docs saying "value x by default (unless overriden by config`)".

But since stdio is an instantiation of the Serial class that we do behind the scenes for the user, having a config option for this makes a lot more sense to me.

[bogdanm]

Not sure what kind of config option you're looking for at this point. Config for what? Default baud rate for all Serial instances? I don't really see how that's better than this PR. Both encourage bad behaviour in a sense (letting people think that declaring a Serial instance without specifying a baud rate is a good idea), but the current PR feels a bit more consistent for the reasons that I already explained. Maybe it's just me.

[geky]

they end up relying on the platform's HAL implementation to choose a baud rate.

Yeah, no, this is bad, I think we all agree this needs to be fixed and you deserve a thanks for getting this pr going.

---

I'm just worried that having the stdio config effect unrelated serial object will unnecessarily confuse users. Before this, modifying the baud rate of your stdio printf had no effect on serial objects on different pins.

Be it good or bad practice, the default baud rate for UART has been conventionally 9600 for a while. The handbook page for Serial even notes this:

The default setting for a Serial connection on the mbed Microcontroller is 9600 baud.

Line 33 seems sufficient for removing the problematic overrides in the targets (though we should probably go remove those anyways).

I still hold that 9600 is a reasonable cross-platform default. However, if we want to require users to be explicit, we can add a constructor that takes the baud-rate and deprecate the non-baud-rate constructors. Maybe it's a bit too early to tell, but this strategy seems to be working for the Thread class.

[0xc0170]

Line 33 seems sufficient for removing the problematic overrides in the targets (though we should probably go remove those anyways).

+1, but that cleanup will be later, first get this in - explicit baud rate in the ctor.

However, if we want to require users to be explicit, we can add a constructor that takes the baud-rate and deprecate the non-baud-rate constructors.

Either that, or do something for our current ctor that might not look good but stdio serial is an exception of its own:

// set default baudrate, for stdio serial we use config, for regular one defaults to 9600 (set in the init list)
if (TX == STDIO_TX) || (RX == STDIO_RX) { //assuming we allow TX=NC or RX=NC
    _baud = MBED_CONF_CORE_STDIO_BAUD_RATE;
}

[bogdanm]

I'm just worried that having the stdio config effect unrelated serial object will unnecessarily confuse users.

You're right, that is confusing. Which is why I think @0xc0170's solution (use MBED_CONF_CORE_STDIO_BAUD_RATE only for Serial pins that use the same UART as the terminal UART) is a better one.

Be it good or bad practice, the default baud rate for UART has been conventionally 9600 for a while.

Or at least it's supposed to. It took me less than 30 seconds yesterday to find #2398 (comment). I'm guessing that's not the only exception.

However, if we want to require users to be explicit, we can add a constructor that takes the baud-rate and deprecate the non-baud-rate constructors.

Definitely, and this was what I initially wanted to do, until I remembered that the Serial constructor looks like this:

Serial(PinName tx, PinName rx, const char *name=NULL);

That name parameter looks quite a bit weird there (plus, I believe it's seldom used), but it still needs to stay there for backward compatibility, and that's exactly where I wanted to put my baud argument. Although I guess there's nothing preventing us from doing this:

Serial(PinName tx, PinName rx, const char *name=NULL, int baud=9600);
Serial(PinName tx, PinName rx, int baud);

(and maybe deprecate the first constructor in the process).

[bridadan]

@bogdanm originally said this:

Before this PR, the default baud rate of a serial object was defined in its HAL implementation. The PR might break some existing code

This actually maybe problematic for Silicon Labs targets since their interface chips are tied to 115200 I believe. If we make this change, you might have to set the default stdio for Silicon Labs targets to be 115200 instead of 9600. @0xc0170 might know more about this.

[sg-]

I don't think STDIO and Serial should share the same macro to define the default speed given that terminal / logging and say GPS or HCI interface etc are more typical use cases of Serial than just logging to terminal IO.

Would a default parameter addition to the constructor of the Serial classes not do the trick?

[bogdanm]

Would a default parameter addition to the constructor of the Serial classes not do the trick?.

It would, but I think we're talking about two different things now:

1. adding a constructor to Serial that has a baud argument.
2. keeping the baud rate consistent (as described in Please update mbed-os to have stdio baud rate configurable #2396).

Fixing 1 above (which I'll do) doesn't automatically solve 2, since I can't force a constructor that'll aways require a baud argument for backward compatibility reasons. I think @0xc0170's suggestion is the best to fix 2:

// set default baudrate, for stdio serial we use config, for regular one defaults to 9600 (set in the init list)
if (TX == STDIO_TX) || (RX == STDIO_RX) { //assuming we allow TX=NC or RX=NC
    _baud = MBED_CONF_CORE_STDIO_BAUD_RATE;
}

[bogdanm]

See #2422 for 1 above.

[sg-]

Isn't MBED_CONF_CORE_STDIO_BAUD_RATE used for stdio retarget baud? I don't think stdio retarget and Serial default baud should share the same default value ie: Serial default baud being 9600 should suffice.

[bogdanm]

This was replaced by #2422.