nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32
https://nodemcu.readthedocs.io
MIT License
7.65k stars 3.12k forks source link

Slight clean up of uart.setup code #3541

Closed jasaw closed 2 years ago

jasaw commented 2 years ago

The current code is a little confusing to read and doesn't return error when given an invalid input. There are 2 code paths here:

  1. uart port is our console. In this case, we accept an optional echo parameter. If echo is not provided, we leave the existing input_echo flag unchanged.
  2. uart port is NOT our console. We accept an optional pins config table parameter.

This PR cleans up the code a bit to make it clearer.

This PR makes changes on top of PR #3540

@jmattsson Please review.

marcelstoer commented 2 years ago

Since you have if (!lua_isnoneornil(L, 6)) { in both the if and the else branch I don't see why this couldn't be the primary condition to check (i.e. guarding the if-else with it).

jmattsson commented 2 years ago

That's a fair point @marcelstoer. It would probably be a bit nicer. I resolved the merge conflict that appeared when I merged the other patches, but I'll hold off merging this one until @jasaw has had a chance to comment.

jasaw commented 2 years ago

@marcelstoer @jmattsson I've moved if (!lua_isnoneornil(L, 6)) { to the outside of the if-else block as suggested.

jmattsson commented 2 years ago

Cheers!