Skip to content

Add driver design page #451

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 7 commits into from
Apr 22, 2025
Merged

Add driver design page #451

merged 7 commits into from
Apr 22, 2025

Conversation

soypat
Copy link
Contributor

@soypat soypat commented Apr 7, 2025

recently was inspired to start working on protips for people designing drivers.

Still a WIP.

Would love a review and suggestions from driver developer extraordinaires @aykevl @deadprogram @ysoldak @sago35

@amken3d
Copy link

amken3d commented Apr 8, 2025

Nice. This would hav helped me when I was writing the TMC drivers. I like the practical aspect of the write up. You should do something similar for tips and tricks for more tinygo topics.

@ysoldak
Copy link
Contributor

ysoldak commented Apr 8, 2025

I like "To abstract machine.Pin one can define type PinOutput func(level bool) and type PinInput func() bool functional interfaces."
Another alternative is for us to have drivers.Pin, but that's a separate discussion and I don't think we come to any decision yet? Also, do we expect pins be pre-configured or a driver's Config method to set them up?
Anyway, the core advise is to avoid machine package, that's good 👍
We can always update docs later when we decide on pins.

@ysoldak
Copy link
Contributor

ysoldak commented Apr 8, 2025

@soypat
Copy link
Contributor Author

soypat commented Apr 8, 2025

@ysoldak I've applied your suggestions- thank you so much!

- Store HAL required for peripheral operation here such as communication buses (I2C, SPI drivers) and pins
- **Avoid**: using TinyGo `machine` package constructs in your driver (read as *don't import `"machine"`*). Users of the Go language (not only TinyGo) also consume the drivers package and they can't compile a program that uses the `"machine"` package. To abstract `machine.Pin` one can define `type PinOutput func(level bool)` and `type PinInput func() bool` functional interfaces.

- Define a `Config` struct for peripherals with many configuration options. See example of a Config struct in action: https://github.com/soypat/lora/blob/main/lora.go
Copy link
Member

Choose a reason for hiding this comment

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

I would point to a driver under the tinygo-org repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes! I've deleted that link and added a list of compliant drivers below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That said, sadly all the drivers are mine :(. Be great to start working on making other existing drivers compliant so we can expand the list

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we have resolved this question yet, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which question?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find other examples which do a good job to sticking to the guidlines except for the ones added in 7905de7

Copy link
Contributor

@ysoldak ysoldak left a comment

Choose a reason for hiding this comment

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

Ship it!

@deadprogram deadprogram changed the base branch from release to dev April 12, 2025 09:30
@soypat
Copy link
Contributor Author

soypat commented Apr 13, 2025

Found a couple typos and in the heat of it added a concurrency section which I felt was missing based on my experiences programming and using async drivers in TinyGo.

@soypat
Copy link
Contributor Author

soypat commented Apr 17, 2025

Last call for comments before merging! Will wait until Monday :)

@soypat soypat merged commit ebdd023 into dev Apr 22, 2025
4 checks passed
@soypat soypat deleted the driver-design branch April 22, 2025 02:04
@soypat
Copy link
Contributor Author

soypat commented Apr 22, 2025

Mergerino the pull requesterino

@soypat
Copy link
Contributor Author

soypat commented Apr 22, 2025

Thank you @ysoldak, @conejoninja for the feedback <3

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.

5 participants