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 busy() for Teensy4 #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

profezzorn
Copy link

Looks like there is a missing return statement.
This PR fixes that.

@Satak
Copy link

Satak commented Dec 8, 2024

May I ask @PaulStoffregen why this PR was never merged? I asked copilot about this function and it gave similar suggestion.

The function int OctoWS2811::busy(void) has a potential bug due to an empty statement after the first if condition. Here is the problematic code:

if (!dma3.complete()) ; // DMA still running
if (micros() - update_begin_micros < numbytes * 10 + 300) return 1; // WS2812 reset
The first if condition checks if dma3 is complete but does nothing because of the semicolon (;). This means the function does not properly check if DMA is still running. Removing the semicolon should fix the issue:

if (!dma3.complete()) return 1; // DMA still running
if (micros() - update_begin_micros < numbytes * 10 + 300) return 1; // WS2812 reset
Now, the function will return 1 if DMA is still running or if the WS2812 reset time has not elapsed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants