Skip to content

Softuart module setup function parameters bug fix #3348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 16, 2020

Conversation

vsky279
Copy link
Contributor

@vsky279 vsky279 commented Dec 13, 2020

Fixes no reported issue.

Make sure all boxes are checked (add x inside the brackets) when you submit your contribution, remove this sentence before doing so.

  • This PR is for the dev branch rather than for the release branch.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/*.

The following code is failing thought it was inline with documentation:

s = softuart.setup(115200, 3, nil)
s = softuart.setup(115200, 3)

The first call was failing as the third argument had to be a number or none. nil was not acccepted (but should be).
The second call failed as softuart did not accept to be setup rx only.

@marcelstoer marcelstoer added this to the Next release milestone Dec 14, 2020
@nwf
Copy link
Member

nwf commented Dec 15, 2020

LGTM

@galjonsfigur
Copy link
Member

Also looks good to me - when I was making #3104 I somehow missed this. Thanks for fixing this! Those kind of bugs and regressions could easily be found by simple tests - I wish I have time to write them especially in the lights of PR like #3324

@marcelstoer marcelstoer merged commit f4ba635 into nodemcu:dev Dec 16, 2020
@vsky279 vsky279 deleted the c_softuartargfix branch December 16, 2020 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants