Skip to content
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

Fix missing return code for ws2812_init() #1816

Merged
merged 1 commit into from
Feb 24, 2017

Conversation

devsaurus
Copy link
Member

Fixes #1815.

  • This PR is for the dev branch rather than for master.
  • 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/en/*.

Like all Lua functions, ws2812_init() must return number of pushed elements onto stack.

Small corrections for ws2812.init() API doc formatting.

@devsaurus devsaurus changed the title Fix missing return code for ws2812_init()- Fix missing return code for ws2812_init() Feb 23, 2017
to Lua's serial console as it shares the same UART and PIN.
- `mode` (optional) either `ws2812.MODE_SINGLE` (default if omitted) or `ws2812.MODE_DUAL`

In `ws2812.MODE_DUAL` mode you will be able to handle two strips in parallel but will lose access to Lua's serial console as it shares the same UART and PIN.

#### Returns
`nil`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you just change that in the code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code change tells the caller of ws2812_init() that 0 results are returned on the stack aka nil. Before that, if told to print the result eg, the calling function would pick up a random value (since ws2812_init() didn't set a return value) and thus extract a random number of items from the stack, causing the crash.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for the explanation.

@marcelstoer marcelstoer added this to the 2.0.0-follow-up milestone Feb 24, 2017
@marcelstoer marcelstoer merged commit 8931f09 into nodemcu:dev Feb 24, 2017
@devsaurus devsaurus deleted the fix_ws2812_init branch February 25, 2017 11:04
eiselekd pushed a commit to eiselekd/nodemcu-firmware that referenced this pull request Jan 7, 2018
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.

2 participants