Peripheral APIs

[Jaana Burcu Dogan]

Hello all,

I am willing to contribute a set of peripheral APIs (e.g. GPIO, SPI, I2C, etc) to the exp package in the upcoming months. The community has already contributed good amount work in this scope and some of them are wide-used (gobot, embd) but there are certain areas I would like to improve.

- We should implement and distribute the surface API layer from an official Go package (for now, golang.org/x/exp/peripheral/...) and let the hardware makers distribute their own drivers if they have a hardware specific implementation.

- We should also let the hardware makers to do configuration (e.g. the mapping of GPIO pins).

- image package's strategy could be used here to load driver packages as a side effect. We should distribute commonly used backend drivers for each protocol (e.g. devfs, sysfs) from the official package and encourage the third party contributors to only work on hardware specific drivers rather than distributing their own packages. 

My plan is to experiment with the APIs under x/exp and look for a more permanent location once the work is more mature. We (Brillo) needs this package since we have proprietary drivers for these protocols and rather than releasing yet another GPIO package and fragment the community, I think we have opportunity to fix the fragmentation.

Looking for your feedback.

jbd

[minux]

What would be the final location of these packages? I imagine that x/exp is just a temporary home for them.

What about x/hardware? I think Dave's serial port package also belongs there.

And the terminal handling packages should go to x/sys instead of x/term.

[Jaana Burcu Dogan]

What would be the final location of these packages? I imagine that x/exp is just a temporary home for them.

x/exp is temporary.

 

What about x/hardware? I think Dave's serial port package also belongs there.

If we have more packages that would fit into the hardware category in the long term, x/hardware sounds good. Serial port may still fit under x/hardware/peripheral though.

I am also thinking whether peripheral is a good name. "comms" would have been another alternative.

[Brad Fitzpatrick]

"peripheral" is a mouthful.

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[minux]

On Mar 24, 2016 10:08 PM, "Brad Fitzpatrick" <brad...@golang.org> wrote:
> "peripheral" is a mouthful.

What about x/hardware/spi or x/io/spi?

[Brad Fitzpatrick]

Those are both shorter and easier to spell.

[Jaana Burcu Dogan]

x/io/spi sounds good to me.

[Jaana Burcu Dogan]

I am also moving the experimental packages to x/exp/io.

[Benny Siegert]

On Fri, 25 Mar 2016 14:30:44 -0700
Jaana Burcu Dogan <j...@golang.org> wrote:

> I am also moving the experimental packages to x/exp/io.

Have you looked at goserial at all? Or at contributing to it?

--
Benny Siegert <bsie...@gmail.com>

[r...@hybridgroup.com]

After some thought, here are some possible answers to the questions I posed in my previous post.

What would be very useful to the community, would be defining some standard general interfaces for GPIO, SPI, serial, & Bluetooth LE, etc. Especially the latter.

Frameworks such as Gobot could easily consume these interfaces. HW vendors could provide implementations for their devices for these well-known interfaces, or the community could do so.

Either way, the device-specific code could then be reused within various applications, again such as those written using Gobot, or just directly used from any Golang code.

Hopefully this makes sense. I look forward to hearing thoughts from others.

[r...@hybridgroup.com]

Hello, everyone. This certainly sounds interesting.

One initial question I have: how does this add something new of value to the community? What about contributing to goserial, gobot and other already existing projects in the community, as already suggested by Benny Siegert? How does this impact/interoperate with existing projects?

On Saturday, March 26, 2016 at 7:41:29 AM UTC-7, Benny Siegert wrote:

[Jaana Burcu Dogan]

To contribute to some of the points here...

As a consumer of several packages in this field, I can say that writing portable code is hard if you are depending on multiple projects. gobot has a concept of adapters/drivers, embd uses imports with side effects, some projects require the use of build tags, etc. Building is hard.

There are so many standalone packages (e.g. stianeikeland/go-rpio) out there with only a single backend implementation. I have seen people has to rewrite it all if they would like to move to another platform where the standalone package's backend is not supported.

If we have a consortium and agree on working on official packages, there will be better chances that our packages will be maintained in the longer term. And third party package/driver maintainers can focus on something more useful than reinventing the wheel for the thousandth time.

I can use the fragmentation in HTTP packages in Python as an analogy for GPIO packages in Go. Go is great when we agree and combine efforts around a single API as a community. Interoperability and compatibility is a major goal.

Most third party package can benefit from readability improvements. Rather than contributing them separately and breaking each project, I think we can solve the API layer for once, we can focus on higher-level packages such as hardware-specific drivers.

[Jaana Burcu Dogan]

On Sat, Mar 26, 2016 at 7:41 AM, Benny Siegert <bsie...@gmail.com> wrote:

On Fri, 25 Mar 2016 14:30:44 -0700
Jaana Burcu Dogan <j...@golang.org> wrote:

> I am also moving the experimental packages to x/exp/io.

Have you looked at goserial at all? Or at contributing to it?

Serial port is somewhat legacy. I dont think we will export a serial API surface from x/io initially but we can use serial to talk to virtual serial devices to write a backend driver (e.g. USB).

 

--
Benny Siegert <bsie...@gmail.com>

[raphael....@gmail.com]

As a past user of gobot, I can confirm that it's API is (at least, was, 6 months ago) not Go-like at all.

2 symptoms :

(a) it required to initialize several objects (connection + device + robot) before doing anything, with unclear lifecycle between them

(b) it wrapped the application process lifecycle

These 2 symptoms are exactly the ones we have known in some heavy framework languages (e.g., Java Spring, Java Servlet engines), where the framework author makes it so easy to have your app developed inside his/her framework that the reverse is increasingly unnatural or subject to code verifications to make sure you are not breaking a hidden assumption of the library.

I am not criticizing gobot, it works well, only saying it looks more like a port to Go of a framework from another language than idiomatic, micro-modular Go, as we have come to love with the http standard library.

The community would certainly benefit from an idiomatic Go API for external hardware, on which gobot or others could add micro-modular drivers, or, if they really want and think it's beneficial, full frameworks.

[Andrew Gerrand]

On 29 March 2016 at 05:17, Jaana Burcu Dogan <j...@golang.org> wrote:

I can use the fragmentation in HTTP packages in Python as an analogy for GPIO packages in Go. Go is great when we agree and combine efforts around a single API as a community. Interoperability and compatibility is a major goal.

This sounds like a great initiative. Want to file a proposal issue? Seems like there are a few domain experts who could contribute to the design. The first thing to do, as I see it, is to nail down the scope.

 

[Jaana Burcu Dogan]

I would like to be more confident about the APIs before the proposal, hence I think x/exp/io is a nice playground for now. I will definitely will file a proposal once the APIs are more mature. I am looking forward to retrieve more feedback before we agree on anything.

[Andrew Gerrand]

Fair enough.

[Manlio Perillo]

It's not really legacy.

On Linux, the kernel has a device driver that enable to access an ANT [1] USB dongle via serial API.  And using the serial API is  more simple than using the USB API.

[1] https://www.thisisant.com/

Manlio 

[Jaana Burcu Dogan]

I was not clear in my previous reply.

The question is whether there is a high demand for a serial port implementation, so that we can consider it in the scope of x/io.

If the demand is to have USB, we should have a USB implementation rather than a USB over virtual serial port backend.

 

Manlio 

[Jaana Burcu Dogan]

On Mon, Mar 28, 2016 at 9:36 PM, Andrew Gerrand <a...@golang.org> wrote:

Fair enough.

I will file a proposal once the APis are mature.

[David Norton]

The question is whether there is a high demand for a serial port implementation, so that we can consider it in the scope of x/io.

There are still industries with serial devices deployed in production. I have written Go apps that are running on factory production lines communicating via serial with high speed bar code scanners and expiration date printers. Also, walk into any restaurant or big box retailer and you're likely to find peripheral devices communicating serially (receipt printers, kitchen video, PIN pads, drive through order display, MSR, etc.). I think it would be useful to include serial in x/io.

[minux]

On Mar 30, 2016 12:27 AM, "Jaana Burcu Dogan" <j...@golang.org> wrote:
> If the demand is to have USB, we should have a USB implementation rather than a USB over virtual serial port backend.

I'm definitely interested in having a pure Go devusbfs interface in x/io/usb that implements the libusb API interface, but in idiomatic Go.

I'd also like a USB gadget package in x/io/usb/gadget.

I think we also need a Bluetooth hci package in x/io.

[sperber...@googlemail.com]

I like the idea of pushing an api! thank you for going that way.

today i saw your first contributions to exp/io/spi and have a question.
Why is a Transfer api better than a Reader/Writer interface?

here: https://github.com/quinte17/spi
i did a hackup a few months ago so someone could try it out.
the drawback of my approach is that you always need to read after a write to clean up unneeded bytes.
an example hw driver which is using it isnt public yet and i dont have the time to reach a "running" state in the next months..
but reading and writing to and from my device is working.

in any case. if this project finally works out i would port my drivers to it..

i hope dave chaneys i2c reader/writer gets in there too!

yours
Stephan

[minux]

On Wed, Mar 30, 2016 at 12:16 PM, sperberstephan via golang-dev <golan...@googlegroups.com> wrote:

I like the idea of pushing an api! thank you for going that way.

today i saw your first contributions to exp/io/spi and have a question.
Why is a Transfer api better than a Reader/Writer interface?

SPI is full duplex, so it doesn't really fit the Reader/Writer interface.

Because the read/write in one SPI transaction is logically one thing,

I don't think it's wise to break them up into two functions calls just

to satisfy existing Go interfaces.

[Nigel Tao]

On Sat, Mar 26, 2016 at 8:30 AM, Jaana Burcu Dogan <j...@golang.org> wrote:
> I am also moving the experimental packages to x/exp/io.

I'm bikeshedding here, and I don't have a better suggestion, but to me
(somebody not familiar with peripherals), x/io or x/exp/io sounds like
it'd be similar to the io or io/ioutil packages in the standard
library, but that's not the case IIUC.

[Andrew Gerrand]

I share those concerns, but I think we can bikeshed the name after it moves from exp.

[Jaana Burcu Dogan]

I was thinking about x/pio (peripheral io). I share the same concern.

minux suggested x/hardware but peripherals are not solely about hardware, so I am not a big fan :)

[Brad Fitzpatrick]

I don't like io, pio, or profiteroles. Actually, profiteroles are tasty, and I actually understand that a profiterole is, unlike a peripheral.

[David Symonds]

On 31 March 2016 at 10:28, Jaana Burcu Dogan <j...@golang.org> wrote:

> minux suggested x/hardware but peripherals are not solely about hardware, so
> I am not a big fan :)

What peripherals are not hardware? Isn't that the very definition of a
peripheral, namely a bit of hardware that isn't part of the computer
itself?

[Jaana Burcu Dogan]

A peripheral device should be an external device. Hardware could be anything physical including the the computer which is running the program's itself. It doesn't tell me much about the scope of these packages.

x/drivers could be another option.

[zel...@gmail.com]

The computer running the program itself is also hardware. Seems like a perfectly fine pre-full-bikeshedding name :-)

[mura li]

Now that you mentioned Bluethooth...

The user-space half of the Bluetooth stack (BlueZ) is really something perfectly fit to be written in Go.

[Jaana Burcu Dogan]

On Wed, Mar 30, 2016 at 9:21 AM, minux <mi...@golang.org> wrote:

On Wed, Mar 30, 2016 at 12:16 PM, sperberstephan via golang-dev <golan...@googlegroups.com> wrote:

I like the idea of pushing an api! thank you for going that way.

today i saw your first contributions to exp/io/spi and have a question.
Why is a Transfer api better than a Reader/Writer interface?

SPI is full duplex, so it doesn't really fit the Reader/Writer interface.

Because the read/write in one SPI transaction is logically one thing,

I don't think it's wise to break them up into two functions calls just

to satisfy existing Go interfaces.

SPI can be used half duplex too. At some point, I think we should add Writer and Reader like half duplex methods.

 

here: https://github.com/quinte17/spi
i did a hackup a few months ago so someone could try it out.
the drawback of my approach is that you always need to read after a write to clean up unneeded bytes.
an example hw driver which is using it isnt public yet and i dont have the time to reach a "running" state in the next months..
but reading and writing to and from my device is working.

 

--

[Kamil Kisiel]

For what it's worth, in most processor reference manuals, devices like I2C, SPI, and even GPIO are referred to as "peripherals", so why not x/peripherals ?

[Dave Cheney]

Well, for one thing, it's a lot longer to type.

[Kamil Kisiel]

x/periph ?

[Raphael Geronimi]

x/peri?

Envoyé de mon iPhone

You received this message because you are subscribed to a topic in the Google Groups "golang-dev" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-dev/ofaaIJPWRKg/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-dev+...@googlegroups.com.

[Dave Cheney]

(To the list this time)

x/io maybe?

[Andrew Gerrand]

Can we bikeshed the repo name once jbd proposes the move from exp?

By that stage we'll know what will actually be in the repository, which should inform any naming decisions.

[nerda...@google.com]

As long as we're bikeshedding...

I like the term "device", even though that pretty much covers all computer hardware including the computer itself. The word seems to fit in my view. Also it's shorter and easier to spell than peripheral. Also there's a common abbreviation to "dev", but that is also commonly used for development or developer, so that's probably not a good idea unless disambiguated, like "x/io/dev".

[Manlio Perillo]

x/io/i2c

x/io/spi

x/io/gpio

x/io/usb

x/io/bt

x/io/bte

x/io/ant

By the way, I'm working on a full ANT stack implementation in Go, including ANT+ profiles, FIT and ANTFS.

Manlio

[minux]

Another thing worth considering is how do we test

the peripheral API packages.

We might need a few dedicated builders with some

peripherals attached.

[Jaana Burcu Dogan]

We are thinking about having a logic analyzer plugged to a raspberry pi to test devfs implementations.

[ha...@currant.com]

We're building a bluetooth framework over at https://github.com/currantlabs/bt which provides separate packages for hci, l2cap, gatt etc. It's still under active development, but when it's finished we'd be happy to contribute it to x/io/bt.

[Ron Evans]

Hi, Burcu

I've been following your commits, looking really cool.

Where is the best place to provide specific feedback on interfaces etc? For example, regarding your comment in commit d5aecea7ffa68142f6a4cdc95761c25c135cf607

"Investigate if command-less read/write is valid."

There are a few oddball devices for which the trigger for the i2c data is some data line on the device itself. For example, the "Hover". In that case, the reading operation does not writes any bytes for the register before peforming a read.

An opposite case would be one of n-DOF devices (I cannot recall which right this moment) that requires sending the register along with some additional command bytes before performing the read.

Perhaps having both a direct Read(), as well as the helpful and often necessary ReadFromRegister(). Likewise Write() and WriteToRegister().

Thanks for listening!

Regards,
Ron

[Jaana Burcu Dogan]

On Tue, Apr 19, 2016 at 2:10 PM, Ron Evans <r...@hybridgroup.com> wrote:

Hi, Burcu

I've been following your commits, looking really cool.

Where is the best place to provide specific feedback on interfaces etc? For example, regarding your comment in commit d5aecea7ffa68142f6a4cdc95761c25c135cf607

Please feel free to file issues on the tracker, prefixed with x/exp/io. https://github.com/golang/go/issues

 

"Investigate if command-less read/write is valid."

There are a few oddball devices for which the trigger for the i2c data is some data line on the device itself. For example, the "Hover". In that case, the reading operation does not writes any bytes for the register before peforming a read.

An opposite case would be one of n-DOF devices (I cannot recall which right this moment) that requires sending the register along with some additional command bytes before performing the read.

Perhaps having both a direct Read(), as well as the helpful and often necessary ReadFromRegister(). Likewise Write() and WriteToRegister().

I was thinking about removing the register argument (named unfortunately cmd in the package) and having only Read and Write available. But I liked the idea of having utility functions that will accept a register argument.

What about ReadReg and WriteReg? They sound more compact.

We can expose these methods from golang.org/x/exp/io/i2c.Device only and keep the driver interface small by asking implementers to only provide Read and Write methods.

 

Thanks for listening!

Thanks so much for your feedback.

[tvone...@gmail.com]

Has there been progress on this since the last commits a few months ago? I'm doing work on kidoman/embd (trying to revive it) and I'm wondering whether to join efforts somehow. I'm heavily using SPI and gpio w/interrupts in embd and so far things work well, but I've seen some initialization issues with interrupts which I haven't narrowed down (could well be the kernel driver).

[Jaana Burcu Dogan]

+ maruel

Marc-Antonie is iterating on the APIs on his own. I am not sure when he will be able to upstream the changes.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.

[Marc-Antoine Ruel]

Thanks Jaana for the ping,

Hi all!

As a side effect of a personal project, I started working on my own hardware library a few months ago. I've been thinking about this problem-space a lot. The library currently live inside another project that you can ignore for the context of this discussion; the initial idea was inspired by Brad's Christmas lights but that's out of scope.

There's several goals I wanted to achieve:

- Make it idiomatic go code:

  - constructors return concrete types

  - functions accept interfaces

  - leverage standard interfaces like io.Writer where it makes sense

  - no interface{}

  - no factories

  - minimal Init() dance yet no behind-your-back init() code

  - have everything compile on all OSes then have OS specific thunk as needed. See the _arm.go and _linux.go files.

- Make it significantly more powerful than current libraries.

- Make it testable

  - include fakes for host buses and device interface

  - nclude self-test to confirm the library physically works.

- Make it a proper HAL (hardware abstraction layer) for bus connectivity, so that it is hardware agnostic while still being able to leverage the hardware in the most efficient way.

- Do not abstract more than absolutely needed.

  - Yet well compartmentalized.

- Performance:

  - Each device driver has a fast-path io.Writer implementation (see apa102, ssd1306, tm1637).

    - yet implements an image.Image compatible devices.Display which makes trivial to scroll lines of an image to apa102 lights or an animated GIF to a ssd1306 display.

  - Leverage memory mapped GPIO registers for both bcm283x and allwinners CPUs to get access to internal pull resistors *and* sysfs gpio based edge triggering detection for the best of both worlds.

- Include read-to-use executables to be able to play with devices. pins is particularly useful.

- I wanted it designed, not organically grown.

- Apache v2 license; I didn't want GPL.

It is a side goal to be able to upstream it or at least make it generally useful for others.

---

A library like kidoman/embd doesn't fit the goals above. Use of factories, (too) flat and crowded packages and interfaces, no reusable fake, abuse of interface{}, no leverage of /dev/gpiomem or /dev/mem. Not anything against the project but it didn't make sense to go to the authors and tell them to rewrite their whole library, breaking all current users (!?) So I decided to write a new one and see if it's actually valuable in retrospect.

I had issues with golang.org/x/exp/io/ because:

- it has the "factory feeling" with the openers. More often than not, a simple function call is sufficient.

- I don't understand why spi.Devfs and i2c.Devfs accept paths, as the kernel doc makes it clear that the paths (i2c, spi) are predictable. This is creating unnecessary knobs.

- spi/driver.Conn let the device driver change too much things. IMHO it's not to the device driver to change the CS line or the speed to communicate at (you are welcome to disagree with me).

- bus interfaces should not have a Close() method. The clients of a bus have no say to close the bus, it's who who created the object in the first place who owns the object and who should close it.

- I've put Bus "factories" in their relevant package by their source of interface, i.e. sysfs or bitbang, not by their protocol.

Instead I create one single host.Bus interface. Since I2C requires specific device addressing, it has an trivial adapter to convert a host.I2C to host.Bus. This is because for I²C, it's generally the device driver that knows the device address, while for SPI, it's the calling app that knows which CS line to use. Compare ssd1306.MakeI2C() and ssd1306.MakeSPI(). tm1637.Make() and bitbang.MakeSPI() are examples of pure pin based communication.

The i2cdev.Dev is what adds all the ReadReg() style function, not the bus itself. It's pure raw code, not yet-another-interface. That keeps the host.I2C interface clean and it makes it easier to write the fake hosttest.I2C.

But like with embd, I felt I arrived too late and wasn't in a position to tell others that their code is not in the right way™ so I felt it was better to try it out by myself and see if there's actual value.

My main fear is figuring out what is being really good design vs what is just being design zealot.

---

The current state:

- It works on a Raspberry Pi and a Pine64. Will make it work on Orange Pi as soon as I receive it.

- It's mostly in the state where I am generally happy about it.

- There's still many known issues[1].

- I break it from time to time.

- Breaking API changes still happen as I'm still looking for better designs.

- I'm now open to comments for improvements and pull requests.

- I'm open into core changes to make it upstreamable to any relevant project.

- I'm mostly ready to at least move it to its own repository. While this library was written by me on my own time and money, I may give the copyright back to my employer (Google). Undecided.

The main question is how can I mutate it to make it acceptable for users while keeping the properties above and if there's actual value in doing so. The current design is incompatible with how exp/io is laid out. Can it be reconciled?

[1] host contains a lot of IR related junk that should be moved, PinIn doesn't have a clear with do cancel Edges() channel, Edges() channel returned is not one-way, PinIn needs a new method "Pull() Pull" to return the current input pull on supported platforms, bitbang implementations are known to be broken, driving a ssd1306 via SPI doesn't work, I haven't committed differential ssd1306 display update yet, host/internal/ has too much junk exported from previous experiments, pins/README.md still list hardware specific things that are not true anymore, etc.

Thanks and sorry for the wall of text,

M-A

[Thorsten von Eicken]

Marc-Antoine, thanks for the wall of text!  I very much share the goals you state. I started with embd because I didn't want to reinvent the wheel and needed SPI, gpio, and interrupts real fast to implement an RFM69 radio driver. The good news is that it works beautifully. The bad news is that the library structure is not what it needs to be. I contacted the author to see whether he's open to let someone else move it forward and I'm now a committer on the repo but I'm not sure where that's really going. I found that embd's SPI and GPIO interfaces, which is what code using embd actually cares about, are very pleasant to use. But the top-level package has all this driver junk in it, etc. So my plan was to try and refactor, keeping these key interfaces, and changing everything else around. I was hoping to be able to keep all the sensor and other device code this way with minimal changes.

An area that I've been particularly stumped by is testing. I'm less concerned about the testing of the bus drivers than about the testing of attached devices. One approach I've considered is to create a test board that can easily be attached to most any embedded computer and that contains a small assortment of popular sensors. It shouldn't be too hard to host one that can run test as part of a travis build. The other approach I've thought of is to add a recording feature to the low-level bus layer so the author of a device driver can record a test run that uses the real device and then the recording can be used to perform future automated test runs. I didn't notice a lot of _test files in your repo ;-). I do like hosttest, maybe some mocking support or recording support could be added.

Overall after spending a couple of hours browsing your work I'm impressed. If you would break it out into its own repo (on github I hope) I'd take a stab at porting my work to see the rubber meet the road... At a high level the things I wasn't thrilled by are the following:

• I'd really like to see a top-level mapping of names to interfaces, what I mean by that is that a program should be able to take a string provided by the user that corresponds to the host's or OS's naming convention and turn that into a handle onto a bus or pin. So if the documentation or the convention on a particular platform is to talk about "the SPI2 bus" then there should be a way to pass "SPI2" (or "spi2") into a commandline flag and have the code go from there. As far as I can tell (...) this fails at two levels. One is that for I2C and SPI the code embeds import paths that lock down which access method is to be used for the bus, e.g. sysfs vs bitbang. The other is that other than the host.Headers.All() mapping there is no general string -> device handle mapping (that I saw).
• Related to the first point, I would really like the same binary to be able to run on rPi, ODROID or CHIP. At least the source code shouldn't have to be changed to swap out import paths.
• The I2C and SPI interfaces are pretty sparse. On the one hand minimal is good, on the other I do find the convenience methods exposed by embd very handy and they save beginners from a bunch of head-scratching. I mean all the Read and Write methods in https://godoc.org/github.com/tve/embd#I2CBus but I realize this is a matter of taste in the end.
• I'm not a fan of the internal packages. I get that something outside of the library shouldn't reach in there, but it also prevents exposing very platform specific things or being able to easily tinker with stuff that's in there. You may not have a use-case for it but that doesn't mean there are no such use-cases. I guess the question could be posed differently: what does the "internal" gain? (I think the only thing it gains is that it's not obvious at the outset where one would start using the library and so it hides those portions, but that's a really bad argument.)

Details I noticed while browsing the code:

• The host package has some type with very generic names that are very special purpose, for example, the Key and Message types that are very IR specific, or the Mode type that is very SPI specific.
• There doesn't seem to be a way to get an error for a PinIn or a PinOut, I'm not sure that's good.
• PinIn should have a way to change the pull (although that can be added once someone implements it).
• For PinIn.Edges() I do not understand "It is important to stop the querying loop by sending a Low to the channel to stop it. The channel will then immediately be closed."
• PinOut is too limited: it needs pull resistor control and there should be a way to set the initial value when initializing the pin.
• In host.sysfs the constructors for I2C and SPI take a bus parameter that is not documented and where it's not obvious which values are available on the current host
• I'm not sure why "Make" is used for constructors instead of "New"
• I didn't understand what you wrote about SPI speed. The rfm69 radio I'm writing a device driver for can run at up to 10Mhz. Why is it not the role of the device driver to set that bus speed? (Maybe the linux sysfs driver doesn't allow different devices on one bus to have different speeds? Is that a reason not to support something like that?)
• host/internal/allwinner needs a more specific name, for example, C.H.I.P uses an allwinner R8 processor and I don't think much carries over

Ugh, I didn't intend to respond with a wall of text, but here I am. I think overall as far as I can tell you achieved your goals, which is no mean feat. Please let me know whether you're going ahead with breaking your library out and whether you'd be interested in a CHIP port.

[Marc-Antoine Ruel]

Thanks a lot for the comments, it's highly appreciated!

Also, I want to clarify that I was very impressed by the other work I referenced in my email; they were useful when fleshing out my own design.

2016-09-20 2:44 GMT-04:00 Thorsten von Eicken <tvone...@gmail.com>:

Marc-Antoine, thanks for the wall of text!  I very much share the goals you state. I started with embd because I didn't want to reinvent the wheel and needed SPI, gpio, and interrupts real fast to implement an RFM69 radio driver. The good news is that it works beautifully. The bad news is that the library structure is not what it needs to be. I contacted the author to see whether he's open to let someone else move it forward and I'm now a committer on the repo but I'm not sure where that's really going. I found that embd's SPI and GPIO interfaces, which is what code using embd actually cares about, are very pleasant to use. But the top-level package has all this driver junk in it, etc. So my plan was to try and refactor, keeping these key interfaces, and changing everything else around. I was hoping to be able to keep all the sensor and other device code this way with minimal changes.

An area that I've been particularly stumped by is testing. I'm less concerned about the testing of the bus drivers than about the testing of attached devices. One approach I've considered is to create a test board that can easily be attached to most any embedded computer and that contains a small assortment of popular sensors. It shouldn't be too hard to host one that can run test as part of a travis build. The other approach I've thought of is to add a recording feature to the low-level bus layer so the author of a device driver can record a test run that uses the real device and then the recording can be used to perform future automated test runs. I didn't notice a lot of _test files in your repo ;-). I do like hosttest, maybe some mocking support or recording support could be added.

That's a great idea. I've implemented recording and playback for I²C and used it with 2 device drivers (bme280 and ssd1306, which is currently in a sad state). I've also made the tool bme280 generate a recording.

With this, all the devices drivers have minimal tests except tm1637, which is trickier to test due to pin bitbanging.

Overall after spending a couple of hours browsing your work I'm impressed. If you would break it out into its own repo (on github I hope) I'd take a stab at porting my work to see the rubber meet the road... At a high level the things I wasn't thrilled by are the following:

I had intentionally not put it yet in its own package because I wanted to keep the leisure to clean it up and do breaking changes. It also made it simpler for me to update my own code. This will happen soonish, based on the interest.

  

• I'd really like to see a top-level mapping of names to interfaces, what I mean by that is that a program should be able to take a string provided by the user that corresponds to the host's or OS's naming convention and turn that into a handle onto a bus or pin. So if the documentation or the convention on a particular platform is to talk about "the SPI2 bus" then there should be a way to pass "SPI2" (or "spi2") into a commandline flag and have the code go from there. As far as I can tell (...) this fails at two levels. One is that for I2C and SPI the code embeds import paths that lock down which access method is to be used for the bus, e.g. sysfs vs bitbang. The other is that other than the host.Headers.All() mapping there is no general string -> device handle mapping (that I saw).

Bitbanging implies no bus number.

Right now there's only one other known way, which is sysfs. Since there's no way to do interrupt based work in user mode, there's no other way in practice to access the I²C and SPI linux drivers.

That said on other platforms, there would be other ways but I'm wary of generalizing anything until there's at least 2 implementations.

• Related to the first point, I would really like the same binary to be able to run on rPi, ODROID or CHIP. At least the source code shouldn't have to be changed to swap out import paths.

That's currently the case IFF you only use linux. It's compatible between debian based distributions.

• The I2C and SPI interfaces are pretty sparse. On the one hand minimal is good, on the other I do find the convenience methods exposed by embd very handy and they save beginners from a bunch of head-scratching. I mean all the Read and Write methods in https://godoc.org/github.com/tve/embd#I2CBus but I realize this is a matter of taste in the end.

host.I2C does not implement host.Bus on purpose as is meant to be used via i2cdev.Dev: it saves from continuously typing the device address and presents ReadReg() in a fully generic way, without having to have 5 different functions.

On the other hand, host.SPI implements Bus on purpose, which inherits io.Writer, which makes it trivial to dump to it, like apa102 is doing.

The unstated thing here is that host.Bus is not a bus but really a point-to-point connection. Probably worth renaming host.Bus accordingly (?) and i2cdev.Dev should be made more prominent / obvious in its usefulness.

• I'm not a fan of the internal packages. I get that something outside of the library shouldn't reach in there, but it also prevents exposing very platform specific things or being able to easily tinker with stuff that's in there. You may not have a use-case for it but that doesn't mean there are no such use-cases. I guess the question could be posed differently: what does the "internal" gain? (I think the only thing it gains is that it's not obvious at the outset where one would start using the library and so it hides those portions, but that's a really bad argument.)

Less is more. Simplicity is hard.

Initially everything was in a flat namespace. I've cargo moved code into internal as I wanted to reduce the "attack surface" until I had figured things out. The idea was to force myself to make proper interfaces so that all the functionality is still available.

I actually liked using pins directly, e.g. bcm283x.GPIO3 or allwinner.PB3, but at the same time this encourage people to write unportable code. I'm undecided about this and open about options. I think in the end most of the code won't be in internal so I do plan to move this. On the other hand, I want to make sure that the project's hierarchy makes it clear that this is not the preferred path.

I added pins.ByFunction() to retrieve a pin by its function. It's probably only useful to return the pin number (?)

 

Details I noticed while browsing the code:

• The host package has some type with very generic names that are very special purpose, for example, the Key and Message types that are very IR specific, or the Mode type that is very SPI specific.

I had listed that in the known issues, this needs to move out.

• There doesn't seem to be a way to get an error for a PinIn or a PinOut, I'm not sure that's good.
• PinIn should have a way to change the pull (although that can be added once someone implements it).

The use case is that you call pin.In() or pin.Out(). If it succeeded, it means that you succeeded in setting the pin in the mode you want and can call Read() / Set() safely from that point on. If it failed, it failed and game over.

The point here is that for in memory mapped drivers, Read() and Set() are literally reading or setting a register. For sysfs gpio, they are writing and reading to an already open handle. Even for sysfs, the chance of failing is nearly nil and not worth bothering. If you ever see a case where reading or writing to /gpioN/value first succeeds then fails, tell me and I'm open to change this but otherwise, this makes the code so much more readable.

I've thought about this a lot. It's a thin line between usability and correctness.

 

• For PinIn.Edges() I do not understand "It is important to stop the querying loop by sending a Low to the channel to stop it. The channel will then immediately be closed."

Outdated comment. Fixed.

• PinOut is too limited: it needs pull resistor control and there should be a way to set the initial value when initializing the pin.

What do you mean by "pull resistor" on an output pin? By definition, an output pin acts as a buffer, most CPUs can source ~20mA. A pull resistor can not source power. Only input pins can have a pull resistor set.

I guess Out() should probably take a parameter but I feared people would continuously call Out() instead of Set().

• In host.sysfs the constructors for I2C and SPI take a bus parameter that is not documented and where it's not obvious which values are available on the current host

 Thanks, I clarified the docstring. Is that good enough?

• I'm not sure why "Make" is used for constructors instead of "New"

I felt that New implies dumb memory allocation and Make implies construction.

New shouldn't return an error. wdyt?

• I didn't understand what you wrote about SPI speed. The rfm69 radio I'm writing a device driver for can run at up to 10Mhz. Why is it not the role of the device driver to set that bus speed? (Maybe the linux sysfs driver doesn't allow different devices on one bus to have different speeds? Is that a reason not to support something like that?)

I've rechecked /usr/include/linux/spi/spidev.h and spi_ioc_transfer does allow to change the speed on a per transaction basis. The problem is that you still need to mangle the interface and can't support io.Writer if the speed is not an invariant.

I'm open to make this device configurable instead of bus configurable.

• host/internal/allwinner needs a more specific name, for example, C.H.I.P uses an allwinner R8 processor and I don't think much carries over

You'd be surprised. Allwinner is relatively consistent with their register mapping at least for the chips I looked at. If you look at R8 user manual (arm7) and compare with the A64 user manual (arm64), you'll see that it uses the same base register of 0x01C20800 for pins in group PB to BH. The main difference is that the R8 doesn't have pins of group PL.

I think the module can be adapted easily, i.e. not fail at

 mem, err := gpiomem.OpenMem(getBaseAddressPL())

and handles this gracefully through the code, hiding this pins group.

Ugh, I didn't intend to respond with a wall of text, but here I am. I think overall as far as I can tell you achieved your goals, which is no mean feat. Please let me know whether you're going ahead with breaking your library out and whether you'd be interested in a CHIP port.

I ordered two CHIP to confirm the support, we'll see in several months since it's "estimated November". It actually could be useful for my project.

Thank you! Your comments were on point.

M-A

[Thorsten von Eicken]

On Tuesday, September 20, 2016 at 1:38:52 PM UTC-7, Marc-Antoine Ruel wrote:

This will happen soonish, based on the interest.

Interest there is! :-)

 

• Related to the first point, I would really like the same binary to be able to run on rPi, ODROID or CHIP. At least the source code shouldn't have to be changed to swap out import paths.

That's currently the case IFF you only use linux. It's compatible between debian based distributions.

Isn't that only true because you're sweeping the two implementations of gpio (mem-mapped vs sysfs) under the rug? So for gpio you hide the difference but for i2c/spi you're exposing the different implementations. Similarly, on the gpio side you're hiding the lack of HW interrupts by burning up CPU cycles (which I forgot to flag as one of the things I'm not a fan of).

• The I2C and SPI interfaces are pretty sparse. On the one hand minimal is good, on the other I do find the convenience methods exposed by embd very handy and they save beginners from a bunch of head-scratching. I mean all the Read and Write methods in https://godoc.org/github.com/tve/embd#I2CBus but I realize this is a matter of taste in the end.

host.I2C does not implement host.Bus on purpose as is meant to be used via i2cdev.Dev: it saves from continuously typing the device address and presents ReadReg() in a fully generic way, without having to have 5 different functions.

On the other hand, host.SPI implements Bus on purpose, which inherits io.Writer, which makes it trivial to dump to it, like apa102 is doing.

The unstated thing here is that host.Bus is not a bus but really a point-to-point connection. Probably worth renaming host.Bus accordingly (?) and i2cdev.Dev should be made more prominent / obvious in its usefulness.

I guess my point is that I find that the I2C and SPI interfaces in embd lead to more readable code. They're not minimal, but sometimes expressivity also counts.

 

Initially everything was in a flat namespace. I've cargo moved code into internal as I wanted to reduce the "attack surface" until I had figured things out. The idea was to force myself to make proper interfaces so that all the functionality is still available.

Makes sense,  I'm just thinking that in the end it does get in the way of special projects and the overall structure needs to be intuitive or documented enough so users naturally do the normal stuff the "right way".

I added pins.ByFunction() to retrieve a pin by its function. It's probably only useful to return the pin number (?)

Yup, that looks nice for GPIO. I expect that I would do something like "redLed := pins.ByFunction("XIO-2"); redLed.Out(); redLed.Set(true);" The "XIO-2" string is gpio pin 2 on CHIP and would have to be configured by the user. The gain here is that (a) ByFunction can accept the strings that are shown in the CHIP manual and tutorials, (b) I get to name the pin however I like in my program (redLed in this example).

 

• There doesn't seem to be a way to get an error for a PinIn or a PinOut, I'm not sure that's good.
• PinIn should have a way to change the pull (although that can be added once someone implements it).

The use case is that you call pin.In() or pin.Out(). If it succeeded, it means that you succeeded in setting the pin in the mode you want and can call Read() / Set() safely from that point on. If it failed, it failed and game over.

The point here is that for in memory mapped drivers, Read() and Set() are literally reading or setting a register. For sysfs gpio, they are writing and reading to an already open handle. Even for sysfs, the chance of failing is nearly nil and not worth bothering. If you ever see a case where reading or writing to /gpioN/value first succeeds then fails, tell me and I'm open to change this but otherwise, this makes the code so much more readable.

I've thought about this a lot. It's a thin line between usability and correctness.

Yup, having every function return an error just because pollutes code everywhere. Q: assuming sysfs, what happens if some other app unexports a pin I'm using? Or also opens the pin and changes its direction?

I'm not necessarily arguing that these are cases that warrant the addition of error returns everywhere, but at the same time it would be good to understand what happens and document it, ideally in a way where this can be caught somehow.

• PinOut is too limited: it needs pull resistor control and there should be a way to set the initial value when initializing the pin.

What do you mean by "pull resistor" on an output pin? By definition, an output pin acts as a buffer, most CPUs can source ~20mA. A pull resistor can not source power. Only input pins can have a pull resistor set.

Mhh, most GPIO pins I'm familiar with support open drain/collector outputs. To drive the i2c clock you need an open drain output, for example, 'cause the slave can do clock stretching. In this case the internal pull-up can be turned on and may or may not suffice. If the gpio does not support a true open drain output this can be simulated by setting the pin value to 0 and turning the output on to output low, and turning the output off to output a high. In those cases the "write" operation turns into a direction switch instead of setting the output value. Mhh, I'm sure you know all this, we must be mis-communicating somewhere?

 

I guess Out() should probably take a parameter but I feared people would continuously call Out() instead of Set().

 

Mhhh, maybe it should be Out(drvier driverType, initial Level) error, where driverType is TotemPole, OpenDrain, OpenDrainPullup (mhh, is totem-pole what's used for cmos?).

• I'm not sure why "Make" is used for constructors instead of "New"

I felt that New implies dumb memory allocation and Make implies construction.

New shouldn't return an error. wdyt?

Is there precedent for this in the stdlib? What you describe is what went though my head, so you did convey the right thing to me, but I was wondering whether this is idiomatic go or not.

• I didn't understand what you wrote about SPI speed. The rfm69 radio I'm writing a device driver for can run at up to 10Mhz. Why is it not the role of the device driver to set that bus speed? (Maybe the linux sysfs driver doesn't allow different devices on one bus to have different speeds? Is that a reason not to support something like that?)

I've rechecked /usr/include/linux/spi/spidev.h and spi_ioc_transfer does allow to change the speed on a per transaction basis. The problem is that you still need to mangle the interface and can't support io.Writer if the speed is not an invariant.

I'm open to make this device configurable instead of bus configurable.

 

I think that would be a good step and probably sufficient for almost all cases. Thanks for checking (I was too lazy).

• host/internal/allwinner needs a more specific name, for example, C.H.I.P uses an allwinner R8 processor and I don't think much carries over

You'd be surprised. Allwinner is relatively consistent with their register mapping at least for the chips I looked at. If you look at R8 user manual (arm7) and compare with the A64 user manual (arm64), you'll see that it uses the same base register of 0x01C20800 for pins in group PB to BH. The main difference is that the R8 doesn't have pins of group PL.

I think the module can be adapted easily, i.e. not fail at

 mem, err := gpiomem.OpenMem(getBaseAddressPL())

and handles this gracefully through the code, hiding this pins group.

Thanks for checking! 

Ugh, I didn't intend to respond with a wall of text, but here I am. I think overall as far as I can tell you achieved your goals, which is no mean feat. Please let me know whether you're going ahead with breaking your library out and whether you'd be interested in a CHIP port.

I ordered two CHIP to confirm the support, we'll see in several months since it's "estimated November". It actually could be useful for my project.

OK, I think I should stop theorizing. What I'd like to do is to port pio to CHIP and port my rfm69 driver (SPI) to it. That's the only way for me to separate theory from practice :-). I'm not in a rush, I still have to improve the interrupt handling and stuff like that that is independent. Can you set some expectation for when it would make sense for me to do this work? I'm not looking for a finished library or anything stable, I just think it would be dumb for me to fork and then you do a major reorg two days later...

Thanks for sharing your work!

[Marc-Antoine Ruel]

bcc: all individuals, keeping ML on cc

2016-09-21 0:19 GMT-04:00 Thorsten von Eicken <tvone...@gmail.com>:

On Tuesday, September 20, 2016 at 1:38:52 PM UTC-7, Marc-Antoine Ruel wrote: 

Isn't that only true because you're sweeping the two implementations of gpio (mem-mapped vs sysfs) under the rug? So for gpio you hide the difference but for i2c/spi you're exposing the different implementations.

It's kinda true. I guess what would be needed is one host.MakeSPI(), one host.MakeI2C() then you call back in the OS specific implementation. In the end it's a factory, like how pins is implemented. The fact it's a factory is hidden to the user though.

But making it a factory blind to the implementation is non trivial; you may have to pass the handle of a kernel driver, GPIO pins to use (like bitbang), bus number for sysfs, etc.

Will think about that to refactor but as I said, I'm not a fan of abstracting until there's at least two implementations which would benefit from the exact same constructor.

 

Similarly, on the gpio side you're hiding the lack of HW interrupts by burning up CPU cycles (which I forgot to flag as one of the things I'm not a fan of).

That's not true. The gpio side transparently leverages sysfs under the hood. It's the best of both world: setup the pull resistor via gpio memory mapped registers, then leverage sysfs to trigger on edge detection. Look at Pin.edge for both allwinner and bcm283x. Interestingly on allwinner only a subset of the pins support edge detection.

There's no busy loop at all.

I couldn't find any other library doing that (please tell me if there's one).

I guess my point is that I find that the I2C and SPI interfaces in embd lead to more readable code. They're not minimal, but sometimes expressivity also counts.

embd encourages you to write inefficient code. :/ Having a bus implement a large interfaces makes creating new implementations or fakes much harder than necessary.

I've already included i2cdev.Dev.ReadReg(), what else that you'd want to see is missing?

Initially everything was in a flat namespace. I've cargo moved code into internal as I wanted to reduce the "attack surface" until I had figured things out. The idea was to force myself to make proper interfaces so that all the functionality is still available.

Makes sense,  I'm just thinking that in the end it does get in the way of special projects and the overall structure needs to be intuitive or documented enough so users naturally do the normal stuff the "right way".

I don't consider the project to be in that state yet. I'm thinking about:

pio/host/

- drivers/

  - allwinner/

  - bcm283x/

  - sysfs/

  - bitbang/

- hal/

  - cpu/

  - hosttest/

  - pins/

  - headers/

  - pine64/

  - rpi/

- internal/

  - gpiomem/

Actually it's too late, I've already pushed the change. :)  wdyt?

I don't want to put pine64 and rpi into drivers they are basically lookup tables. That's why I've split them from their corresponding processor in the first place.

 

I added pins.ByFunction() to retrieve a pin by its function. It's probably only useful to return the pin number (?)

Yup, that looks nice for GPIO. I expect that I would do something like "redLed := pins.ByFunction("XIO-2"); redLed.Out(); redLed.Set(true);" The "XIO-2" string is gpio pin 2 on CHIP and would have to be configured by the user. The gain here is that (a) ByFunction can accept the strings that are shown in the CHIP manual and tutorials, (b) I get to name the pin however I like in my program (redLed in this example).

I've added pins.ByName(), as ByNumber() can be tricky in some cases, like GPIO number on Allwinner processors.  So you are probably looking at

pins.ByName("PB8") or something like that. I don't know about CHIP. "XIO-2" doesn't sound like a function to me but a name.

 

Yup, having every function return an error just because pollutes code everywhere. Q: assuming sysfs, what happens if some other app unexports a pin I'm using? Or also opens the pin and changes its direction?

I'm not necessarily arguing that these are cases that warrant the addition of error returns everywhere, but at the same time it would be good to understand what happens and document it, ideally in a way where this can be caught somehow.

host.PinIn / PinOut documents that using Read() or Set() without having respectively called In() or Out() has undefined behavior. I implemented the comments there.

That's true that other apps could mess up by unexporting behind your back but I feel that's a bit over the top at that point. The library itself never unexport a pin so if you know what stuff runs on your host, I don't see the point. I mean, I'm pretty confident nobody would look at the error code anyway.

• PinOut is too limited: it needs pull resistor control and there should be a way to set the initial value when initializing the pin.

What do you mean by "pull resistor" on an output pin? By definition, an output pin acts as a buffer, most CPUs can source ~20mA. A pull resistor can not source power. Only input pins can have a pull resistor set.

Mhh, most GPIO pins I'm familiar with support open drain/collector outputs. To drive the i2c clock you need an open drain output, for example, 'cause the slave can do clock stretching. In this case the internal pull-up can be turned on and may or may not suffice. If the gpio does not support a true open drain output this can be simulated by setting the pin value to 0 and turning the output on to output low, and turning the output off to output a high. In those cases the "write" operation turns into a direction switch instead of setting the output value. Mhh, I'm sure you know all this, we must be mis-communicating somewhere?

AFAIK in GPIO parlance, this is an pin setup as input with a pull down resistor. It's acting as a weak output. Two pins are connected together, both as input, one with a pull resistor, one floating. The one floating reads the signal from the other pin. See pioselftest for the physical testing of this functionality. 

  

I guess Out() should probably take a parameter but I feared people would continuously call Out() instead of Set().

 

Mhhh, maybe it should be Out(drvier driverType, initial Level) error, where driverType is TotemPole, OpenDrain, OpenDrainPullup (mhh, is totem-pole what's used for cmos?).

No, totem pole is not used for cmos AFAIK but my electronics knowledge dates from 15 years ago.

embd has similar functions with PUllDown and PullUp, except that both functions accept an interface{} instead of being methods. Then if you look at the actual implementation, it always return an error and it is not implemented. I'm surprised because the code was written in 2014.

It's effectively impossible to write an I²C bitbanging driver without input pull resistors. On Allwinner, this is quite rapid to do but on BCM, the caller has to waste 150 cycles to set pull resistors and I still haven't figured a good way to burn exactly 150 cycles in Go.

• I'm not sure why "Make" is used for constructors instead of "New"

I felt that New implies dumb memory allocation and Make implies construction.

New shouldn't return an error. wdyt?

Is there precedent for this in the stdlib? What you describe is what went though my head, so you did convey the right thing to me, but I was wondering whether this is idiomatic go or not.

$ git grep "func New"  -- "src/*.go" | wc -l

226

$ git grep "func Make"  -- "src/*.go" | wc -l

18

There's clearly a preference towards New. I don't know if someone who knows the history more here can shed some light on this. I've renamed the functions to NewXXX() for coherency with the stdlib even if I'm not a fan.

 

• I didn't understand what you wrote about SPI speed. The rfm69 radio I'm writing a device driver for can run at up to 10Mhz. Why is it not the role of the device driver to set that bus speed? (Maybe the linux sysfs driver doesn't allow different devices on one bus to have different speeds? Is that a reason not to support something like that?)

I've rechecked /usr/include/linux/spi/spidev.h and spi_ioc_transfer does allow to change the speed on a per transaction basis. The problem is that you still need to mangle the interface and can't support io.Writer if the speed is not an invariant.

I'm open to make this device configurable instead of bus configurable.

 

I think that would be a good step and probably sufficient for almost all cases. Thanks for checking (I was too lazy).

Done. Hilariously in a previous refactor I had completely removed the speed handling and hadn't realized. Oh well. It's fixed now and I verified with an oscilloscope.

 

OK, I think I should stop theorizing. What I'd like to do is to port pio to CHIP and port my rfm69 driver (SPI) to it. That's the only way for me to separate theory from practice :-). I'm not in a rush, I still have to improve the interrupt handling and stuff like that that is independent. Can you set some expectation for when it would make sense for me to do this work? I'm not looking for a finished library or anything stable, I just think it would be dumb for me to fork and then you do a major reorg two days later...

 

I've committed R8 support, albeit I can't test it.

Thanks for sharing your work!

You're welcome!

M-A

[Thorsten von Eicken]

On Wednesday, September 21, 2016 at 1:09:50 PM UTC-7, Marc-Antoine Ruel wrote:

But making it a factory blind to the implementation is non trivial; you may have to pass the handle of a kernel driver, GPIO pins to use (like bitbang), bus number for sysfs, etc.

Will think about that to refactor but as I said, I'm not a fan of abstracting until there's at least two implementations which would benefit from the exact same constructor.

Agreed that this is not about doing some premature quick fix.

Similarly, on the gpio side you're hiding the lack of HW interrupts by burning up CPU cycles (which I forgot to flag as one of the things I'm not a fan of).

That's not true. The gpio side transparently leverages sysfs under the hood. It's the best of both world: setup the pull resistor via gpio memory mapped registers, then leverage sysfs to trigger on edge detection. Look at Pin.edge for both allwinner and bcm283x. Interestingly on allwinner only a subset of the pins support edge detection.

There's no busy loop at all.

Ooops, sorry, I think I misread your implementation as I was browsing through the code. Apologies.

I couldn't find any other library doing that (please tell me if there's one).

I'm curious why you decided to implement the edge detection yourself instead of letting the sysfs driver do it, which allows the function to pushed down into the HW. You ask sysfs to generate an interrupt on both rising and falling edges. That's 2x the necessary interrupts. Then you read the value, which takes extra time (specially when gpiomem is not available). And the fact that you do the edge detection means that a pulse that is shorter than the interrupt response time (allthe way into user-space is missed).

 

I guess my point is that I find that the I2C and SPI interfaces in embd lead to more readable code. They're not minimal, but sometimes expressivity also counts.

embd encourages you to write inefficient code. :/ Having a bus implement a large interfaces makes creating new implementations or fakes much harder than necessary.

By inefficient code, do you mean the fact that a WriteReg implementation would have to copy the array being written in order to prepend the register number? Given that you're going through a system call afterwards, does that matter? Do you expect the caller to do any better by not exposing such a helper function?

 

Actually it's too late, I've already pushed the change. :)  wdyt?

 

My head is spinning :-) (Looks good at first glance, but I need more time)

WRT pull-up/pull-down resistors and all that, let me try a fresh start. I'm looking at https://www.kernel.org/doc/Documentation/gpio/sysfs.txt and in the section on "direction" is says:

Writing as "out" defaults to initializing the value as low. To ensure glitch free operation, values "low" and "high" may be written to configure the GPIO as an output with that initial value.

I know that turning a pin to an output starting at a known value without a glitch is something I've needed in the past. I believe that a good library should support that. I think you could either add an initial param to Out() or you could allow Set() before out and that saves the value in the object so it can start out with the right value when Out() is called. The latter is definitely what one would do at the register level on most uC's (i.e. set the output value and then turn the output section on).

If you want to look at the bcm2835 docs, https://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-Peripherals.pdf, page 95 table 6-8 the legend clearly states

If the GPIO pin is being used as in input (by default) then the value in the SET{n} field is ignored. However, if the pin is subsequently defined as an output then the bit will be set according to the last set/clear operation. 

In terms of performance, doesn't the fact that you combine the pull-up specification with the In() function cause a performance hit? For example, in the bitbang I2C implementation https://github.com/maruel/dlibox/blob/master/go/pio/host/drivers/bitbang/i2c.go#L169 instead of just doing a quick register update to switch the pin from output to input you are doing the whole 2x 150 cycles dance to set the pull-up. When you implement the clock stretching 8 lines up you'll do this for every bit. Note how the bcm2835 chip has the pull-up/pull-down completely separate from direction, you can even enable the pull-up if you use an alternate function.

It's effectively impossible to write an I²C bitbanging driver without input pull resistors. On Allwinner, this is quite rapid to do but on BCM, the caller has to waste 150 cycles to set pull resistors and I still haven't figured a good way to burn exactly 150 cycles in Go.

This is because you're not separating the pull-up/pull-down control from the direction control.

Cheers!

[Zellyn]

On Tuesday, September 20, 2016 at 4:38:52 PM UTC-4, Marc-Antoine Ruel wrote:

I felt that New implies dumb memory allocation and Make implies construction.

New shouldn't return an error. wdyt?

fwiw, in Go generally, `t := time.time{}` or `var mu sync.Mutex` imply dumb memory allocation, and `New` implies construction.

[Jaana Burcu Dogan]

What do you mean by "factory feeling"? You construct an opener and pass it only at Open. This is crazily simply if you compare what relevant libraries are trying to achieve abstractions over abstractions. Given the portability requirement, I ask you write a single line of Opener construction :)

 

- I don't understand why spi.Devfs and i2c.Devfs accept paths, as the kernel doc makes it clear that the paths (i2c, spi) are predictable. This is creating unnecessary knobs.

Because these are only predictable and a proper implementation shouldn't require that you have capabilities to run i2cdetect. Until we have have a proper Go implementation of i2cdetect, we should be any opinionated on caring about calculating any paths.

 

- spi/driver.Conn let the device driver change too much things. IMHO it's not to the device driver to change the CS line or the speed to communicate at (you are welcome to disagree with me).

A Conn implementation is a proper implementation of the protocol on a specific env. It implements every aspect of the protocol. Changing the CS or the speed is a basic requirement to implement SPI. Who's going to implement the calls to change these things? What's your replacement proposal? I am not sure I can totally follow this item.

 

- bus interfaces should not have a Close() method. The clients of a bus have no say to close the bus, it's who who created the object in the first place who owns the object and who should close it.

That's why there are not called busses but connections to busses. You are entitled to close the connection to a bus not the bus' itself. The original package don't have any concept of busses for that reason. Buses are something belongs to the system but a connection is owned by the client program.

 

- I've put Bus "factories" in their relevant package by their source of interface, i.e. sysfs or bitbang, not by their protocol. 

There are some boards that only specifically have a bus for SPI but nothing else. How are we support such devices with a centralized host definition.

 

Instead I create one single host.Bus interface. Since I2C requires specific device addressing, it has an trivial adapter to convert a host.I2C to host.Bus. This is because for I²C, it's generally the device driver that knows the device address, while for SPI, it's the calling app that knows which CS line to use. Compare ssd1306.MakeI2C() and ssd1306.MakeSPI(). tm1637.Make() and bitbang.MakeSPI() are examples of pure pin based communication.

The i2cdev.Dev is what adds all the ReadReg() style function, not the bus itself. It's pure raw code, not yet-another-interface. That keeps the host.I2C interface clean and it makes it easier to write the fake hosttest.I2C.

But like with embd, I felt I arrived too late and wasn't in a position to tell others that their code is not in the right way™ so I felt it was better to try it out by myself and see if there's actual value.

My main fear is figuring out what is being really good design vs what is just being design zealot.

This is the main challenge in this experimentation. If a good design is convergable, we should commit work on the project. If not, we live on by living in our own packages and stop wasting more brain cycles on a pio package.

[Marc-Antoine Ruel]

2016-09-22 3:01 GMT-04:00 Thorsten von Eicken <tvone...@gmail.com>:

On Wednesday, September 21, 2016 at 1:09:50 PM UTC-7, Marc-Antoine Ruel wrote:

I'm curious why you decided to implement the edge detection yourself instead of letting the sysfs driver do it, which allows the function to pushed down into the HW. You ask sysfs to generate an interrupt on both rising and falling edges. That's 2x the necessary interrupts. Then you read the value, which takes extra time (specially when gpiomem is not available). And the fact that you do the edge detection means that a pulse that is shorter than the interrupt response time (allthe way into user-space is missed).

I could implement the optimization of reading from gpiomem when doing edge detection. I want to write a perf test before doing so.

Forcing interrupts on both edge is the only way to detect spurious reads, and I've seen this happen.

I guess my point is that I find that the I2C and SPI interfaces in embd lead to more readable code. They're not minimal, but sometimes expressivity also counts.

embd encourages you to write inefficient code. :/ Having a bus implement a large interfaces makes creating new implementations or fakes much harder than necessary.

By inefficient code, do you mean the fact that a WriteReg implementation would have to copy the array being written in order to prepend the register number? Given that you're going through a system call afterwards, does that matter? Do you expect the caller to do any better by not exposing such a helper function?

I agree it doesn't matter. Added more functions but this has an intrinsic assumption that the memory is represented as big endian.

WRT pull-up/pull-down resistors and all that, let me try a fresh start. I'm looking at https://www.kernel.org/doc/Documentation/gpio/sysfs.txt and in the section on "direction" is says:

Writing as "out" defaults to initializing the value as low. To ensure glitch free operation, values "low" and "high" may be written to configure the GPIO as an output with that initial value.

Done.

I know that turning a pin to an output starting at a known value without a glitch is something I've needed in the past. I believe that a good library should support that. I think you could either add an initial param to Out() or you could allow Set() before out and that saves the value in the object so it can start out with the right value when Out() is called. The latter is definitely what one would do at the register level on most uC's (i.e. set the output value and then turn the output section on).

Done. I've ripped out Set().

 

In terms of performance, doesn't the fact that you combine the pull-up specification with the In() function cause a performance hit? For example, in the bitbang I2C implementation https://github.com/maruel/dlibox/blob/master/go/pio/host/drivers/bitbang/i2c.go#L169 instead of just doing a quick register update to switch the pin from output to input you are doing the whole 2x 150 cycles dance to set the pull-up. When you implement the clock stretching 8 lines up you'll do this for every bit. Note how the bcm2835 chip has the pull-up/pull-down completely separate from direction, you can even enable the pull-up if you use an alternate function.

This code is known to be broken.

I'm a bit confused by the presence of pine64 and rpi in host/hal. I thought "hal" meant the platform independent abstraction, the ones I should zoom in on and use. As opposed to host/driver, which is the platform-specific abstractions that I should ignore, unless I'm implementing a new platform. Hence I'd move pine64 and rpi into driver. But maybe I'm off track...

Moved both pine64 and rpi into drivers/

 

I also find devices/i2cdev a little odd. I totally get the separation of the driver providing a bus and i2cdev providing access to one device on the bus, but I would expect to find i2cdev in host. It's one of the core abstractions that the library exposes, isn't it?

Moved. In fact, I refactored the whole thing again and extracted the interfaces into protocols. This is the way I found to get away from the import cycles.

 

The fact that to get started with GPIO one has to dive down into host/hal/pins (or host/hal/headers) is a bit unintuitive. There's not much at higher levels to guide someone there. Not sure I have a good suggestion other than move hal/pins & hal/headers into host and move the rest of what's in hal into drivers. What would happen if you hoisted all the .go files in host to the top level? This way the functions that the user should use would be right there at the top-level of the library.

That's true. I removed 'hal' completely. I moved pin constants to protocols/pins (this sounds weird though).

I finally added the magic host.NewI2C() and host.NewSPI() as you had asked. To make this work, I augmented i2c.Bus and spi.Bus to be able to query the actual pin used. This complicated the import orders a lot but I think it was worth it.  i2c.Dev now makes more sense than the previous i2cdev.Dev.

This makes simple utilization trivial. Here's an actual execution:

~$ go run a.go

23.580°C

~$ cat a.go

package main

import (

"fmt"

"github.com/maruel/dlibox/go/pio/devices"

"github.com/maruel/dlibox/go/pio/devices/bme280"

"github.com/maruel/dlibox/go/pio/host"

)

func main() {

bus, _ := host.NewI2C()

b, _ := bme280.NewI2C(bus, bme280.O1x, bme280.O1x, bme280.O1x, bme280.S500ms, bme280.FOff)

env := &devices.Environment{}

b.Read(env)

fmt.Printf("%6.3f°C\n", float32(env.MilliCelcius)*0.001)

}

Does that start to look good?

Thanks,

M-A

[Jaana Burcu Dogan]

On Mon, Sep 19, 2016 at 11:44 PM, Thorsten von Eicken <tvone...@gmail.com> wrote:

Marc-Antoine, thanks for the wall of text!  I very much share the goals you state. I started with embd because I didn't want to reinvent the wheel and needed SPI, gpio, and interrupts real fast to implement an RFM69 radio driver. The good news is that it works beautifully. The bad news is that the library structure is not what it needs to be. I contacted the author to see whether he's open to let someone else move it forward and I'm now a committer on the repo but I'm not sure where that's really going. I found that embd's SPI and GPIO interfaces, which is what code using embd actually cares about, are very pleasant to use. But the top-level package has all this driver junk in it, etc. So my plan was to try and refactor, keeping these key interfaces, and changing everything else around. I was hoping to be able to keep all the sensor and other device code this way with minimal changes.

An area that I've been particularly stumped by is testing. I'm less concerned about the testing of the bus drivers than about the testing of attached devices. One approach I've considered is to create a test board that can easily be attached to most any embedded computer and that contains a small assortment of popular sensors. It shouldn't be too hard to host one that can run test as part of a travis build. The other approach I've thought of is to add a recording feature to the low-level bus layer so the author of a device driver can record a test run that uses the real device and then the recording can be used to perform future automated test runs. I didn't notice a lot of _test files in your repo ;-). I do like hosttest, maybe some mocking support or recording support could be added.

Overall after spending a couple of hours browsing your work I'm impressed. If you would break it out into its own repo (on github I hope) I'd take a stab at porting my work to see the rubber meet the road... At a high level the things I wasn't thrilled by are the following:

• I'd really like to see a top-level mapping of names to interfaces, what I mean by that is that a program should be able to take a string provided by the user that corresponds to the host's or OS's naming convention and turn that into a handle onto a bus or pin. So if the documentation or the convention on a particular platform is to talk about "the SPI2 bus" then there should be a way to pass "SPI2" (or "spi2") into a commandline flag and have the code go from there. As far as I can tell (...) this fails at two levels. One is that for I2C and SPI the code embeds import paths that lock down which access method is to be used for the bus, e.g. sysfs vs bitbang. The other is that other than the host.Headers.All() mapping there is no general string -> device handle mapping (that I saw).

My suggestion to this is simply:

func Map(virtual string, physical int)

Then you can override the virtual space with different board configurations depending on what's the host.

[Marc-Antoine Ruel]

Oops our messages crossed.

2016-09-22 17:31 GMT-04:00 Jaana Burcu Dogan <j...@golang.org>:

On Mon, Sep 19, 2016 at 5:18 PM, Marc-Antoine Ruel <mar...@chromium.org> wrote:

- it has the "factory feeling" with the openers. More often than not, a simple function call is sufficient.

What do you mean by "factory feeling"? You construct an opener and pass it only at Open. This is crazily simply if you compare what relevant libraries are trying to achieve abstractions over abstractions. Given the portability requirement, I ask you write a single line of Opener construction :)

Fair enough, I tried to get rid of the struct and see if I could make it work without this in practice.

 

 

- I don't understand why spi.Devfs and i2c.Devfs accept paths, as the kernel doc makes it clear that the paths (i2c, spi) are predictable. This is creating unnecessary knobs.

Because these are only predictable and a proper implementation shouldn't require that you have capabilities to run i2cdetect. Until we have have a proper Go implementation of i2cdetect, we should be any opinionated on caring about calculating any paths.

i2cdetect is about detecting chips on an I²C. I was referring to the bus itself, not the chips. But that's something interesting too.

 

 

- spi/driver.Conn let the device driver change too much things. IMHO it's not to the device driver to change the CS line or the speed to communicate at (you are welcome to disagree with me).

A Conn implementation is a proper implementation of the protocol on a specific env. It implements every aspect of the protocol. Changing the CS or the speed is a basic requirement to implement SPI. Who's going to implement the calls to change these things? What's your replacement proposal? I am not sure I can totally follow this item.

 

- bus interfaces should not have a Close() method. The clients of a bus have no say to close the bus, it's who who created the object in the first place who owns the object and who should close it.

That's why there are not called busses but connections to busses. You are entitled to close the connection to a bus not the bus' itself. The original package don't have any concept of busses for that reason. Buses are something belongs to the system but a connection is owned by the client program.

Will do right after dinner.

- I've put Bus "factories" in their relevant package by their source of interface, i.e. sysfs or bitbang, not by their protocol. 

There are some boards that only specifically have a bus for SPI but nothing else. How are we support such devices with a centralized host definition.

The separation of protocols vs core host implementation now enables this. This wasn't true before.

 

Instead I create one single host.Bus interface. Since I2C requires specific device addressing, it has an trivial adapter to convert a host.I2C to host.Bus. This is because for I²C, it's generally the device driver that knows the device address, while for SPI, it's the calling app that knows which CS line to use. Compare ssd1306.MakeI2C() and ssd1306.MakeSPI(). tm1637.Make() and bitbang.MakeSPI() are examples of pure pin based communication.

The i2cdev.Dev is what adds all the ReadReg() style function, not the bus itself. It's pure raw code, not yet-another-interface. That keeps the host.I2C interface clean and it makes it easier to write the fake hosttest.I2C.

But like with embd, I felt I arrived too late and wasn't in a position to tell others that their code is not in the right way™ so I felt it was better to try it out by myself and see if there's actual value.

My main fear is figuring out what is being really good design vs what is just being design zealot.

This is the main challenge in this experimentation. If a good design is convergable, we should commit work on the project. If not, we live on by living in our own packages and stop wasting more brain cycles on a pio package.

I like seeing in real life the code to get a feeling, I don't mind continuously refactoring it in the meantime. That's the main advantage of writing a library with 0 users.

2016-09-22 17:34 GMT-04:00 Jaana Burcu Dogan <j...@golang.org>:

My suggestion to this is simply:

func Map(virtual string, physical int)

Then you can override the virtual space with different board configurations depending on what's the host.

That's a good idea. I had a really crummy one to work around import cycles. I'll try to do this before end of tomorrow.

M-A

[Thorsten von Eicken]

On Thursday, September 22, 2016 at 2:38:36 PM UTC-7, Marc-Antoine Ruel wrote:

Oops our messages crossed.

2016-09-22 17:31 GMT-04:00 Jaana Burcu Dogan <j...@golang.org>:

On Mon, Sep 19, 2016 at 5:18 PM, Marc-Antoine Ruel <mar...@chromium.org> wrote:

- it has the "factory feeling" with the openers. More often than not, a simple function call is sufficient.

What do you mean by "factory feeling"? You construct an opener and pass it only at Open. This is crazily simply if you compare what relevant libraries are trying to achieve abstractions over abstractions. Given the portability requirement, I ask you write a single line of Opener construction :)

Fair enough, I tried to get rid of the struct and see if I could make it work without this in practice.

I have to admit that this factory discussion is a bit too abstract for me to follow :-). Would it be possible to state a couple of concrete examples & use-cases that should or show not be supported? (Negative examples are good to reach minimalism.)

I believe that over the set of rPi, ODROID, CHIP, Pine64, BBB, I would hope the following can be supported without any code changes:

- get a handle onto a gpio pin called X, where X is the name used in the platform's documentation

- get a handle onto SPI connection on bus N with chip select M, where N and M are numbers as used in the platform's documentation

- get a handle onto I2C connection on bus N to device with address A, where N & A are numbers as documented

- get a handle onto I2C bus N, the use-case being to detect devices on that bus

For each of the above handles, I expect that the expected HW capabilities can be invoked, such as setting bus speed, receiving edge-triggered interrupts, etc. A good question is (a) how to handle missing features in some implementation, and (b) how to handle additional features.

A concrete example for a missing feature might be:

- An SPI or I2C bus implementation that cannot set a speed, or that cannot set the desired speed

- A gpio pin that does not support interrupts

A concrete example for an extra feature that is not handled by the STD interface might be:

- A gpio pin that supports edge triggered interrupts with debouncing

A thought: would it be acceptable to access such extra features by querying the bus/pin handle for implemented interfaces?

Since I mentioned negative examples:

- rule out the capability to query by capability, e.g., can't request "gimme a gpio pin that supports interrupts", "gimme an spi bus that has 2 chip selects", "gimme an spi bus that is unused", ...

Am I missing the point completely?

[ma...@splice.com]

+adrian who's been working on a fork of exp/io.

Adrian, do you care to show/explain what you've done?

On Thursday, September 22, 2016 at 2:38:36 PM UTC-7, Marc-Antoine Ruel wrote:

[Thorsten von Eicken]

On Thursday, September 22, 2016 at 2:32:31 PM UTC-7, Marc-Antoine Ruel wrote:

 

The fact that to get started with GPIO one has to dive down into host/hal/pins (or host/hal/headers) is a bit unintuitive. There's not much at higher levels to guide someone there. Not sure I have a good suggestion other than move hal/pins & hal/headers into host and move the rest of what's in hal into drivers. What would happen if you hoisted all the .go files in host to the top level? This way the functions that the user should use would be right there at the top-level of the library.

That's true. I removed 'hal' completely. I moved pin constants to protocols/pins (this sounds weird though).

I finally added the magic host.NewI2C() and host.NewSPI() as you had asked. To make this work, I augmented i2c.Bus and spi.Bus to be able to query the actual pin used. This complicated the import orders a lot but I think it was worth it.  i2c.Dev now makes more sense than the previous i2cdev.Dev.

This makes simple utilization trivial. Here's an actual execution:

~$ go run a.go

23.580°C

~$ cat a.go

package main

import (

"fmt"

"github.com/maruel/dlibox/go/pio/devices"

"github.com/maruel/dlibox/go/pio/devices/bme280"

"github.com/maruel/dlibox/go/pio/host"

)

func main() {

bus, _ := host.NewI2C()

b, _ := bme280.NewI2C(bus, bme280.O1x, bme280.O1x, bme280.O1x, bme280.S500ms, bme280.FOff)

env := &devices.Environment{}

b.Read(env)

fmt.Printf("%6.3f°C\n", float32(env.MilliCelcius)*0.001)

}

Does that start to look good?

I think you can refactor faster than I can comprehend ;-)

It does look pretty awesome, I must say! The only thing I don't understand is why you don't allow the bus number (and CS number) to be specified for host.NewI2c and host.NewSPI.

One musing I had was whether it's best to put the generic selection logic into 'host' using the Enumerate() function you have, or whether it's better to push the selection down into the device-specific code. Maybe there's no difference. 

[Marc-Antoine Ruel]

Please review https://godoc.org/github.com/maruel/dlibox/go/pio

I significantly changed it, again.

2016-09-22 22:20 GMT-04:00 Thorsten von Eicken <tvone...@gmail.com>:

On Thursday, September 22, 2016 at 2:32:31 PM UTC-7, Marc-Antoine Ruel wrote:

This makes simple utilization trivial. Here's an actual execution:

~$ go run a.go

23.580°C

~$ cat a.go

package main

import (

"fmt"

"github.com/maruel/dlibox/go/pio/devices"

"github.com/maruel/dlibox/go/pio/devices/bme280"

"github.com/maruel/dlibox/go/pio/host"

)

func main() {

bus, _ := host.NewI2C()

b, _ := bme280.NewI2C(bus, bme280.O1x, bme280.O1x, bme280.O1x, bme280.S500ms, bme280.FOff)

env := &devices.Environment{}

b.Read(env)

fmt.Printf("%6.3f°C\n", float32(env.MilliCelcius)*0.001)

}

Does that start to look good?

I think you can refactor faster than I can comprehend ;-)

It does look pretty awesome, I must say! The only thing I don't understand is why you don't allow the bus number (and CS number) to be specified for host.NewI2c and host.NewSPI.

The bus number is purely a host specific concept. These functions will take the first bus available and return it. In the case of SPI, the CS 0 line will be used. In short, it's the "Just give me the damn thing" kind of functions, which is useful for first time users. This is important because on some platforms (like the Raspberry Pi), the I²C bus #0 is disabled by default, training people to hardcode bus number 1 in their code. That's really bad. I guess I could name the function JustGiveMeADamnI2C().

My point is that if the auto-select choses specific pins, there may be no bus number involved. Taking a more concrete example:

The main reason I started the bitbang implementation is that armbian/legacy on pine64/a64 doesn't have an SPI driver. In this case, you want the tool to "take a few pin and make them act as SPI". There's no "bus number" or "chip select number" involved, only good old pin numbers. That's why I added a query for the pins used to spi.Conn, i2c.Conn and uart.Conn. In practice it could try to use the pins that would be expected to be used normally by the kernel driver when sysfs is not exposed but that's irrelevant for the discussion here.

As I said in the initial email, maybe I'm overthinking this. :)

So there's 3 utilization levels:

1. I know what I'm doing and I want this specific implementation, e.g. sysfs.NewSPI(busNumber, CS) and bitbang.NewSPI(clk, mosi, miso, cs).
   
2. I know the concept of bus number but I don't care which driver implements it, host.NewSPI(busNumber, CS)
   
3. I don't care, give me something that works, host.NewSPI() without any arguments. (It could live in protocols directly).
   

I implemented #1 and #3. I agree #2 may be interesting for users.

One musing I had was whether it's best to put the generic selection logic into 'host' using the Enumerate() function you have, or whether it's better to push the selection down into the device-specific code. Maybe there's no difference. 

I implemented an in-process driver registry. The two things that I think make it work are:

- clear priority classes

- way to skip a driver on unrelated platform

- way to return all errors that occurred

It surprisingly work better than I had expected. It is dynamically extensible but doesn't mess with the user experience too much. host.Init() provides a wrapper that does "the right thing" for pre-provided drivers.

2016-09-22 18:28 GMT-04:00 Thorsten von Eicken <tvone...@gmail.com>:

I have to admit that this factory discussion is a bit too abstract for me to follow :-). Would it be possible to state a couple of concrete examples & use-cases that should or show not be supported? (Negative examples are good to reach minimalism.)

I believe that over the set of rPi, ODROID, CHIP, Pine64, BBB, I would hope the following can be supported without any code changes:

- get a handle onto a gpio pin called X, where X is the name used in the platform's documentation

- get a handle onto SPI connection on bus N with chip select M, where N and M are numbers as used in the platform's documentation

- get a handle onto I2C connection on bus N to device with address A, where N & A are numbers as documented

I want to call out that pio separates the bus i2c.Conn and the device as referenced by address i2c.Dev. It is very frequent that I²C have hardcoded addresses (FWIU unlike many SMBus devices which has reprogrammable addresses) so in that case, the device driver accepts an i2c.Conn, not an i2c.Dev.

 

- get a handle onto I2C bus N, the use-case being to detect devices on that bus

For each of the above handles, I expect that the expected HW capabilities can be invoked, such as setting bus speed, receiving edge-triggered interrupts, etc. A good question is (a) how to handle missing features in some implementation, and (b) how to handle additional features.

A concrete example for a missing feature might be:

- An SPI or I2C bus implementation that cannot set a speed, or that cannot set the desired speed

- A gpio pin that does not support interrupts

A concrete example for an extra feature that is not handled by the STD interface might be:

- A gpio pin that supports edge triggered interrupts with debouncing

A thought: would it be acceptable to access such extra features by querying the bus/pin handle for implemented interfaces?

That's a good question. A concrete example: bcm283x supports edge triggering on all pins but Allwinner processors do not. The only way to know if a pin supports it or not on Allwinner is to try it, there's no way to query this information. I tried to make the error message clear about that.

 

Since I mentioned negative examples:

- rule out the capability to query by capability, e.g., can't request "gimme a gpio pin that supports interrupts", "gimme an spi bus that has 2 chip selects", "gimme an spi bus that is unused", ...

At sysfs level, you can open 2 handles and use them concurrently, and the kernel driver will do the arbitrage.

On SPI, you can open /dev/spidev0.0 and /dev/spidev0.1 and bus arbitrage is done by the kernl.

On I²C, you can open /dev/i2cdev-0 and do multiple concurrent Tx(), it's the kernel driver that will handle this. This works because sysfs I²C driver always do transactions as a single kernel call. I use a lock so Close() won't be run while the Tx() is happening but it's not more than that.

 

Am I missing the point completely?

No. :)  They are many pattern and I'm looking at supporting the broadest use cases with the lesser amount of code.

 

M-A

[Marc-Antoine Ruel]

2016-09-23 12:08 GMT-04:00 Marc-Antoine Ruel <mar...@chromium.org>:

So there's 3 utilization levels:

1. I know what I'm doing and I want this specific implementation, e.g. sysfs.NewSPI(busNumber, CS) and bitbang.NewSPI(clk, mosi, miso, cs).
   
2. I know the concept of bus number but I don't care which driver implements it, host.NewSPI(busNumber, CS)
   
3. I don't care, give me something that works, host.NewSPI() without any arguments. (It could live in protocols directly).
   

I implemented #1 and #3. I agree #2 may be interesting for users.

I've now implemented all three. #2 and #3 are at https://godoc.org/github.com/maruel/dlibox/go/pio/host

In the preceding example, you'd have to replace host.NewI2C() with host.NewI2CAuto().

M-A

[Thorsten von Eicken]

On Friday, September 23, 2016 at 9:08:52 AM UTC-7, Marc-Antoine Ruel wrote:

- get a handle onto I2C connection on bus N to device with address A, where N & A are numbers as documented

I want to call out that pio separates the bus i2c.Conn and the device as referenced by address i2c.Dev. It is very frequent that I²C have hardcoded addresses (FWIU unlike many SMBus devices which has reprogrammable addresses) so in that case, the device driver accepts an i2c.Conn, not an i2c.Dev.

Yup. I think there's perhaps another use-case that I overlooked:

• get a handle on an I2C connection to bus N, probe a number of addresses (many devices allow the exact address to be set via pins or have a couple of different part numbers), then obtain an i2c.Dev for the actual device once found.

I believe this is supported by your last refactoring, but I haven't double-checked. I'm just trying to state some concrete use-cases.

A thought: would it be acceptable to access such extra features by querying the bus/pin handle for implemented interfaces?

That's a good question. A concrete example: bcm283x supports edge triggering on all pins but Allwinner processors do not. The only way to know if a pin supports it or not on Allwinner is to try it, there's no way to query this information. I tried to make the error message clear about that.

I think debouncing is a better example, because it's not so common that it would be in protocols.gpio while the edge triggering is supported by the interface. So debouncing would be an example where a specific implementation would support a feature that is not exposed through the portable interface. Concrete example: the Ti AM3359 in the BBB supports debouncing. So I would expect that host.am3359 would have an interface:

type PinDebounce interface {

  Debounce(time.Duration) // sets the debounce duration, use the zero value to disable

}

and host.am3359.Pin would have a function

func (p *Pin) Debounce(t time.Duration) { ... }

and then a client could do something like:

myPin := gpio.ByName("GPIO0")

if myDebouncedPin, ok := myPin.(PinDebounce); ok {

  myDebouncedPin.Debounce(10*time.Millisecond)

}

not 100% pure, but better than interface{} all over the place...

[Thorsten von Eicken]

On Friday, September 23, 2016 at 11:36:11 AM UTC-7, Marc-Antoine Ruel wrote:

I've now implemented all three. #2 and #3 are at https://godoc.org/github.com/maruel/dlibox/go/pio/host

In the preceding example, you'd have to replace host.NewI2C() with host.NewI2CAuto().

FYI, I created a pull-request to support CHIP. At least this way I'm not just complaining from the cheap seats ;-).

The one thing I would change overall is to flatten the host package hierarchy. I've come to expect Go package hierarchies to be shallow, for example, the std net package has lots of types and there aren't net/ip, net/udp, net/tcp, ... sub-packages. So I'd move I2C, SPI, .... all into host (and call the types like I think you previously had, e.g. I2CConn, SPIConn, ...). I don't think the host sub-packages buy or improve anything. Less hierarchy also improves the godoc browsing in my experience. But in the end that's a matter of taste and it's your lib ;-).

[setha...@gmail.com]

I would agree except some popular libraries like py-spidev provide spi.readbytes and spi.writebytes methods. It would seem reasonable to provide both these methods and a transfer method, making the spi a io.readwriter while still providing full duplex capabilities.

On Wednesday, March 30, 2016 at 12:21:36 PM UTC-4, minux wrote:

On Wed, Mar 30, 2016 at 12:16 PM, sperberstephan via golang-dev <golan...@googlegroups.com> wrote:

I like the idea of pushing an api! thank you for going that way.

today i saw your first contributions to exp/io/spi and have a question.
Why is a Transfer api better than a Reader/Writer interface?

SPI is full duplex, so it doesn't really fit the Reader/Writer interface.

Because the read/write in one SPI transaction is logically one thing,

I don't think it's wise to break them up into two functions calls just

to satisfy existing Go interfaces.

here: https://github.com/quinte17/spi
i did a hackup a few months ago so someone could try it out.
the drawback of my approach is that you always need to read after a write to clean up unneeded bytes.
an example hw driver which is using it isnt public yet and i dont have the time to reach a "running" state in the next months..
but reading and writing to and from my device is working.

 

[Thorsten von Eicken]

Do you have examples of SPI devices where readbytes is useful? It seems such a marginal use-case to me that it would be more confusing than anything else to support io.reader. It's not like it really enables anything as far as I know. Maybe I'm just missing something...