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.

Sign up for GitHub

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

Jump to bottom

x/exp/io: distinguish transport layer config from protocol required config #15803

Closed
rakyll opened this issue May 23, 2016 · 6 comments
Closed

x/exp/io: distinguish transport layer config from protocol required config #15803

rakyll opened this issue May 23, 2016 · 6 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@rakyll
Copy link
Contributor

rakyll commented May 23, 2016

Currently, in order to keep the driver implementations extendible enough, driver.Opener interfaces requires no arguments. But it is creating barriers in the reusability of certain configuration fields.

I implemented the following high-level Grove LCD package and running it against i2c.Devfs and a custom driver that talks through a web service.

package grovelcd

import  "golang.org/x/exp/io/i2c/driver"
func New(o driver.Opener) (*GroveLCD, error) 

glcd, err := grovelcd.New(&grovelcd.WebProxy{
    URL: "http://192.168.159.121:8080",
})

glcd, err := grovelcd.New(&i2c.Devfs{Dev: "/dev/i2c-1", Addr: 0x62})

What is not nice is that grovelcd package cannot embed the I2C address required by the device at the device library level since Addr needs to be injected by the transport layer, i2c.Devfs.

We should distinguish such arguments from the driver implementation configurations to allow libraries to encapsulate the protocol details from the users.

The new i2c.Driver should look like:

type Opener interface {
    Open(addr int) (Conn, error)
}

And the user should only care about opening the right bus, address will be embedded in grovelcd package.

glcd, err := grovelcd.New(&i2c.Devfs{Dev: "/dev/i2c-1"})

Thoughts?

/cc @minux @proppy
@rakyll rakyll added this to the Unreleased milestone May 23, 2016
@rakyll rakyll self-assigned this May 23, 2016
@proppy
Copy link
Contributor

proppy commented May 23, 2016

+1 for passing protocol level argument (i.e: addresses), independently from backend specific args (path/to/devfs/device).

@minux
Copy link
Member

minux commented May 24, 2016 via email

@mattetti
Copy link
Contributor

Having to pass the address all the time is confusing for new users and since it's static and related to the i2c device you are targeting. The part I don't quite understand tho is how/why the proxy would know about the address to implement the Opener interface, more specifically the addr argument:

glcd, err := grovelcd.New(&grovelcd.WebProxy{ URL: "http://192.168.159.121:8080", })

Shouldn't the addr be available on the package already and therefore the interface could be simplified to:

type Opener interface { Open() (Conn, error) }

@rakyll
Copy link
Contributor Author

rakyll commented May 27, 2016

Shouldn't the addr be available on the package already and therefore the interface could be simplified to:

This is exactly why we are putting the addr back to Open, so the devfs driver won't need you to pass it. Higher level packages, such as the grovelcd package, can hardcode it and encapsulate from the user.

grovelcd.New(&i2c.Devfs{Dev: "/dev/i2c-1"})

Am I misunderstanding anything?

@mattetti
Copy link
Contributor

No, it was me being confused. It looks great

@gopherbot
Copy link

CL https://golang.org/cl/24215 mentions this issue.

@golang golang locked and limited conversation to collaborators Jun 24, 2017
@rsc rsc unassigned rakyll Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@mattetti @proppy @rakyll @minux @gopherbot