-
Notifications
You must be signed in to change notification settings - Fork 909
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
Netdevdemo #3500
Netdevdemo #3500
Conversation
Same comment I left at the end of my PR (#3452): "You lost me here. How does a driver in the drivers repo get access to non-exported interfaces in the tinygo repo? The drivers have to be written to some interface". I think you'll need to pull the driver PR (#523) also, and update the drivers and test the examples to match the changes you're making to tinygo when proposing alternative ideas. One comment on your PR: it doesn't feel right for every tinygo application that wants to use "net" to import "unsafe". I'm grimacing thinking about writing the user documentation for that requirement. Actually, I'm not understanding the problem your PR is trying to solve, fundamentally. Maybe you can describe a scenario that illustrates the problem before we spend more time in solution space. To set the problem up, imagine my PR went in as-is. Time passes. Now, in the future, an issue comes up but somehow we shot ourselves in the foot with my PR and can't move forward or have to invent some distasteful work-around and live with it. What is the scenario you have in mind? Sorry for being dense; I'm just not coming up with anything. |
Ok, I'm an idiot. I see it now, the driver doesn't need to know the interface definition to implement it. Duh, Go 101. Let me try this out on tinygo/drivers and update the PRs. Sorry for being a knucklehead; I just couldn't see what now seems obvious. Thanks @soypat for being persistent. PS. I don't like this "unsafe" business, but the scale seems tipped. |
OK- so we have TinyGo stdlib, which I'll just call tinygo, the native Go stdlib or upstream for short, and third party libraries, which I'll call libs. So to quickly answer your doubts two key points:
The beauty of this PR is that there's not much more to it. It will allow users to begin developing drivers outside of the tinygo repo in external libs. If you want typed interfaces you can do that on the lib side of things by creating an interface that wraps the The way I see it not much has changed since your first proposed RFC where you had a function called
So the problem this solves is losing compatibility with Go. Why should we take that step if we can avoid it completely? There's also the issue of duplicated types in your PR which introduces another HardwareAddr and Addr types. This PR also elimanates that. The |
Netdev lib with helpful typesHere's an example of a third party library that provides users with your proposed types for the socket(2) interface. If I'm not missing any crucial bit of information, I think we keep the best of all worlds with this approach! We can develop on top of the socket(2) bare-bones implementation however we please and learn more about what works and what does not while keeping compatibility with upstream and tinygo. |
@scottfeldman I just saw your response! Didn't mean to pile on you after you had understood. |
I have one wifinina example working! It'll take me probably all day tomorrow to finish the conversion and test all the examples and devices and update the documentation. But it's looking good. Lots of stuff fell out. |
Ok, I've made tinygo & drivers updates based on this demo, specifically: tinygo repo
drivers repo
|
Now that #3704 has been merged I am going to close this PR. Thanks everyone! |
@scottfeldman @aykevl @bgould @deadprogram
As of now this does not compile throwing a convoluted stream of llvm errors. It serves the purpose to illustrate the artistic direction the netdev PR could take.
To set the netdev it would suffice to do the following in a tinygo program. Note the changes in the interface. There is a working proof of concept attached as well to illustrate at a simple level what is being done and that the core idea works.
I have not been able to actually get this to work due to the errors which seem to be on the net package internal side of things. Probably due to the
net.Addr
type which has some excessively complex methods that may be breaking tinygo. I'll try to reduce thenet.Addr
Proof of concept of linking functions that accept different interfaces with identical method sets
usage of netdever
errors