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

Update pygamer to HAL 0.17 #750

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

ianrrees
Copy link
Contributor

@ianrrees ianrrees commented Aug 27, 2024

Summary

Initial work by @bradleyharden for v2 SERCOM, with more updates to account for improvements in our project and others.

Still some issues to work through, but at this stage all the examples compile:

  • Remove neopixel support (can be reverted once working drivers are available)
  • Decide whether to remove or fix the USB polling example (Fixed)
  • Clean up Git history
  • Update CHANGELOG.md for the BSP

@ianrrees
Copy link
Contributor Author

From some not-very-thorough testing today, I think fixing the timer-based neopixel issues will at least require some work in ws2812-timer-delay-rs because the timing it generates (via GPIO and waiting on a supplied timer) isn't quite right on SAMD51 with the new HAL. I've tried adjusting the library locally, and have the examples working well with opt-level="s", but then dialing it up to 1 or higher causes problems again. On paper, and based on the WS2812 datasheet I found, the current delays in that crate shouldn't work, so I think it's likely that fixing them for this case would just move the problem to some other user of that crate.

Although the SPI neopixel example isn't working at present, that architecture seems a lot more reasonable to me; in particular the timer+GPIO approach would be a stretch to make work on a 48MHz SAMD21, but should be fine using a SERCOM on that part. I'm thinking that the appropriate thing might be to move the PyGamer neopixel examples away from the timer-based crate and to the SPI-based one.

@ianrrees
Copy link
Contributor Author

ianrrees commented Aug 28, 2024

Debugging the SPI-driven WS2812 examples revealed that there are some subtle changes in behaviour of SPI writes between the old and new HAL. The old behaviour doesn't work reliably for me in release profile (the pixels show the rainbow, but flicker quite a bit), and the new behaviour doesn't work at all for WS2812 despite the SPI HAL implementation behaving correctly according to the SpiBus trait AFAICT.

So, at present, I think that the neopixel examples are broken for reasons outside of atsamd-rs. I've opened #752 to track that, since it's not specific to PyGamer although some of the issues are worse with a string of multiple neopixels. The question is whether we want to get those examples working before moving forward with this PR.

My vote is to remove the neopixel examples from PyGamer until we can get them working, and proceed with this PR.

Port the pygamer BSP to the `gpio::v2` and `sercom::v2::spi` APIs. While
implementing this, I discovered that the pygamer uses an undocumented
combination of pins for `Sercom1`. It uses PA17, PB22 & PB23. The
datasheet indicates that PA17 is in `IoSet1`, while the other two pins are
in `IoSet3`. It shouldn't be possible to use these together, but it
appears there is an undocumented `IoSet`. Add this as `UndocIoSet1` for
`Sercom1`.

Also fix a warning from a trinket_m0 example.
@ianrrees ianrrees force-pushed the port_pygamer_to_v2_b branch 6 times, most recently from 71362c4 to 7f711c6 Compare September 6, 2024 01:49
This leaves NeoPixel examples in a broken state, it appears work will
need to be done in WS2812 crates and/or the HAL to get timing working
again.
These compile, but don't currently work reliably, due to changes in the
HAL changing the timing of the asynchronous serial signals to the WS2812
@ianrrees ianrrees changed the title Draft: Update pygamer to current HAL Update pygamer to HAL 0.17 Sep 6, 2024
@jbeaurivage jbeaurivage merged commit f0e9316 into atsamd-rs:master Sep 6, 2024
108 checks passed
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.

3 participants