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

Broadcom pi zero2w neopixel misbehaving/crash fix #7570

Merged
merged 12 commits into from
Feb 14, 2023

Conversation

RetiredWizard
Copy link

These changes fix #5947. I've tested on both the zero2w and the pi4b (The 4b didn't exhibit the original issue) and the boards now behave properly with 1 to 30 pixels. I've run a 30 pixel animation on the zero2w for 8+ hours without a hang.

Original corrupted rgb transmission:
image
Early Fix, not using transmission queue:
image
This PR:
image

These changes result in working neopixel functionality. I've tested on
both the zero2w and the pi4b (The 4b didn't exhibit the original issue)
and the boards now behave properly with 1 to 30 pixels and the board hanging
no longer occurs.

Remove mod that didn't help during testing
Restoring back to original structure
Replace 2 microsecond delay w/deterministic loop
Remove unneded check for empty queue

Put transmit delay outside loop so Queue is used

Make sure last transmission is complete
@RetiredWizard
Copy link
Author

I just tested this on a zero w rather than the zero2w and it works but it looks like the code is getting stuck more frequently than the other tested board in the loop that checks for an empty queue. I'm not sure that's a problem if I reduce the loop abort counter but I want to test on the three models I have. I'll update once I've finished testing.

@RetiredWizard
Copy link
Author

Tested on 4b, zero2w and zero w. Runs without glitches or crashes.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Please use && in your conditions instead of &. & is a bitwise test and && is boolean logic.

ports/broadcom/common-hal/neopixel_write/__init__.c Outdated Show resolved Hide resolved
ports/broadcom/common-hal/neopixel_write/__init__.c Outdated Show resolved Hide resolved
RetiredWizard and others added 3 commits February 13, 2023 14:44
Co-authored-by: Scott Shawcroft <scott@tannewt.org>
Co-authored-by: Scott Shawcroft <scott@tannewt.org>
@RetiredWizard
Copy link
Author

RetiredWizard commented Feb 14, 2023

So I misunderstood what COMPLETE_MEMORY_READS was doing and clearly over used it in the update for the zero w. I've pulled all the calls I added for that update out and I'm testing it now. The only one remaining that I added was at the very end of the code just before the PWM function of the pin is turned off. I feel like that one might be needed but I guess I'll do some tests.

edit: It doesn't look like that last memory barrier was needed either....

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit 0be5397 into adafruit:main Feb 14, 2023
@RetiredWizard RetiredWizard deleted the broadcomNeopix branch February 14, 2023 21:25
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.

Neopixel Broadcom on PiZero1W misbehave or crash
2 participants