I2C WriteBlockData not working as expected

[owenwaller]

Hi,

As discussed in this thread on the golang-nuts list.

I am using GoBot v1.2 that has the rewritten i2c subsystem, to try to drive a PiGlow (based on a SN3218 IC) connected to a Raspberry Pi 1. The code is written using the "metal bot" style as I would like to reuse this as part of another project.

My sample code is available on the Go playground as: https://play.golang.org/p/Xma_6zBtT2

When I run this code it shows no output. None of the error values are non-nil and led zero does not light on the PiGlow.

As far as I can tell I am communicating with the PiGlow over i2c correctly. My code is based on that of Toon Schoenmakers code. This code is not based on the GoBot 12c subsystem. However. Toon's test code works correctly and drives the PiGlow as expected.

Can someone with more experience of GoBots i2c subsystem make a suggestion as to what I might be doing wrong, or if this is a new bug in the i2c subsystem on a RaspPi.

Thanks

Owen

[erkkah]

Hi there.

A difference from Toon's code is that you do a "reset" and then return. I don't know how the PiGlow works, but it seems as you init everything, turn on a light, and then reset everything again before exiting. Don't know what "reset" does, but it's not used in Toon's example.

Possibly the default raspi bus in GoBot is not the same as bus 1 used in the other code. But you should have gotten errors then, I guess.

[owenwaller]

Hi Erkkah,

Firstly, if I take out the reset behaviour (aka the call to PiGlow.Halt()) I see the same behaviour - i.e. nothing. So it's not the reset call that is the problem. Although you are correct that pointing it in does seem a bit pointless if I want the led to stay on.

Secondly there is only one i2c device on the Rasp Pi 1 and its at /dev/i2c-1. If I print the result of Adaptor.GetDefaultBus() call as used in the Adaptor.GetConnection call it will return 1. Toon's code also uses 1 as the i2c bus number.

In fairness, I do believe that on a Rasp Pi 2 and 3 the default i2c bus is in fact 0.

Owen

[deadprogram]

Hi, everyone

I notice that the implementation being used by PiGlow calls to WriteByte() and WriteByteBlock() are basically does the equivalent of what Gobot is doing as WriteByteData() and WriteBlockData().

At least one immediate difference I see, is that the implementation of WriteByte() is simply using the sysfs and not actually calling the SMBUS implementation: https://bitbucket.org/gmcbay/i2c/src/1235f1776ee749f0eaeb6de69d8804a6dd70d9d5/i2c_bus.go?at=master&fileviewer=file-view-default#i2c_bus.go-80

Not sure if that is actually the issue here, but seems worth pointing out.

[erkkah]

Interesting. So I guess one test would be to use the Write method instead of WriteByteData to see if it works any better.

[owenwaller]

Erkkahj,

I'm not 100% sure what you mean here but...

All the WriteXXX, ReadXXX, Write and Read calls in i2c.go are just wrappers around calls to

sysfs.I2cDevice.Write(byte) or sysfs.I2cDevice.Read()

The source is here

Where as Toon's code bypasses sysfs entirely, and writes directly on the SMBUS. Which I think is Ron's point above.

[erkkah]

Hi Owen,

I think it's the other way around, or something like it :)

The WriteXXX and ReadXXX calls in i2c call the corresponding methods in sysfs, which in turn are implemented using the SMBUS ioctl calls.

The Write and Read calls on the other hand use direct file IO (direct I2C).

The library used by Toon's code mixes, and uses SMBUS for the block call and file IO for the WriteByte call.

To test with as little differences as possible, you could try to replace your WriteByteData calls with Write calls. That was all I meant.

[owenwaller]

Erkkah,

Err exactly which type do you want me to call Write on? The Connection type?

Owen

[owenwaller]

Ah ha!

Okay if I take Erkkah's suggestion and swap all of the WriteXXX calls to i2cDevice.Write calls then it works. I get exactly what I expect - no errors and lights in the PiGlow.

My updated code is here: https://play.golang.org/p/-zcc2ODfLZ Which enables LED zero for 5 seconds before turning it off again.

So, if this is a direct write to the i2c bus then it works. But if the WriteXXX calls are via syscalls to the ioctl on the smbus then it doesn't.

So it looks like you syscalls aren't working as you expect. Which at least narrows down the problem.

Owen

[deadprogram]

This is great work, thanks for looking into it!

One question: if you replace only the WriteByteData() with Write(), but left in the WriteBlockData() did that work? Trying to determine of all the syscalls were not working as expected, or only that one, since this is similar to what the gmcbay/i2c package does.

[erkkah]

Yes, great find!

Really interesting if the device supports SMBUS block operations but not the WriteByteData() call.

[owenwaller]

OK so I have just tried this.

In the Start() method if I swap out the 2nd call to Write()

// Start initiates the Driver
func (p *PiGlowDriver) Start() error {
    b := []byte{enableOutput, 0xFF}
    w, err := p.conn.Write(b)
    if err != nil {
        return err
    }
    if w != 2 {
        return errors.New("Start: Failed to write the enableOutput bytes")
    }

    b2 := []byte{enableLeds, 0xFF, 0xFF, 0xFF}
    w, err = p.conn.Write(b2)
    if err != nil {
        return err
    }
    if w != 4 {
        return errors.New("Start: Failed to write the enableLeds bytes")
    }
}

for a call to WriteBlockData() like this:

// Start initiates the Driver
func (p *PiGlowDriver) Start() error {
    b := []byte{enableOutput, 0xFF}
    w, err := p.conn.Write(b)
    if err != nil {
        return err
    }
    if w != 2 {
        return errors.New("Start: Failed to write the enableOutput bytes")
    }
    err = p.conn.WriteBlockData(enableLeds, []byte{0xFF, 0xFF, 0xFF})
    if err != nil {
        return err
    }

    return nil
}

It breaks the code. Nothing happens on the PiGlow after the change to WriteBlockData - no lights and no errors when the code runs.

So whatever is going on WriteBlockData() is broken.

Owen

[owenwaller]

And just to confirm this.

If I swap a call to Write() for a call to WriteByteData() for the enableOutut byte at the start of the Start() method. And then replace the WriteDataBlock() with a Write() again for the enableLeds write in the above code everything still works.

So the combinations of: Write() everywhere works ok WriteByteData() and Write works ok WriteByteData() and WriteBlockData() fails Write() and WriteBlockData() fails

So the issue is defiantly under the call-stack of WriteBlockData.

Owen

[deadprogram]

Thanks @owenwaller for helping track this down. I renamed the issue to better reflect what the problem seems to be here.

Also, please feel free to create a PR with your PIGlow driver, if you get a chance. We would probably however want it to accept a Connector instead of an already existing Connection as the param, which would break your own current code.

[owenwaller]

NP. Once things are fixed I'd be happy to put together a PR for the PiGlow.

I'll open a new issue for it as I need some advice as how best to achieve that.

[erkkah]

I found a difference between the i2c module used in Toon's code and the Gobot implementation in that that the prior always uses the PEC block transaction while Gobot always uses the non-PEC. Added a possible fix in PR #378.

[deadprogram]

@owenwaller if you get a chance to test this, it would be greatly appreciated.

[owenwaller]

So pulling the request with:

git fetch origin pull/378/head:i2c-pec-fix
git checkout i2c-pec-fix

Then trying a quick a dirty test, by swapping the previous and working call to Write() to WriteBlockByte in the Start() method. So from this:

func (p *PiGlowDriver) Start() error {
    err := p.conn.WriteByteData(enableOutput, 0x01)
    if err != nil {
        return err
    }
    b2 := []byte{enableLeds, 0xFF, 0xFF, 0xFF}
    w, err := p.conn.Write(b2)
    if err != nil {
        return err
    }
    if w != 4 {
        return errors.New("Start: Failed to write the enableLeds bytes")
    }
    return nil
}

to this

func (p *PiGlowDriver) Start() error {
    err := p.conn.WriteByteData(enableOutput, 0x01)
    if err != nil {
        return err
    }
    err = p.conn.WriteBlockData(enableLeds, []byte{0xFF, 0xFF, 0xFF})
    if err != nil {
        return err
    }
    return nil
}

The code fails. There are no lights. The first version works, the later doesn't. So WriteDataBlock() is still broken in the PR :(

Owen

[erkkah]

Hi Owen, that's curious, thanks for trying it out.

Can you go directly to WriteBlockData in sysfs/i2c_device.go and hard code it to use I2C_SMBUS_BLOCK_DATA_PEC?

With that, it should be the exact same code as in the other library, it would be interesting to figure out if it's the functionality check that fails or what it is.

/Erik

[owenwaller]

Hi Erkkah,

Err, I can do but I'm not entirely sure what I'm supposed to change in WriteBlockData to enable this. Can you paste in a diff of some sort and I'll make the change?

Owen

[owenwaller]

Err but while I look in sysfs/i2c.go at the `WriteBlockData code, it looks like you are throwing away data, if the function is passed more than 32 bytes. We have this circa line 167...

    if len(data) > 32 {
        data = data[:32]
    }

Which is silently throwing away data. Why? And surely that should fail with an error - the input data byte slice was to large or something similar? In my case I'm only ever passing in 19 bytes, so I'm not falling foul of this. But others might be.

Owen

[erkkah]

Sure, change the WriteBlockData method to this: https://gist.github.com/erkkah/503607981df5c3ed2b74bbde61221a3a

The 32 byte limit comes from the SMBus standard. I agree I could document the 32 byte limiting better. I did not want to change the contract when I changed the implementation during refactoring, but I guess we should return an error for too large requests.

[erkkah]

Looks like I made a mistake when importing constants from the c headers. Really confusing. I have updated the PR, but I need to get a device that behaves consistently with block transfers to test and really fix this. Thanks again Owen for reporting and testing on your end. /Erik

[owenwaller]

Hi Erkkah,

Yeah, it works! And err, I need to eat some humble pie....

So what I have just tried is this:

git fetch origin pull/378/head:i2c-pec-fix
git checkout i2c-pec-fix

I am assuming this includes the updates to the PR.

If I then replace just the Start() method with the version above then I get what I expect. No errors and a light on the PiGlow.

At the minute this is the only Write() that I have replaced with a call to WriteDataBlock().

Let me add calls to WriteDataBlock everywhere else to confirm things, but at the minute it looks like the PR is good and works. Adding the extra error when the data is truncated would be nice, as I didn't actually know there was a 32 byte limit on the data block size.

And now for my humble pie...

Erkkah, I feel I own you an apology. I suspect your PR, may have worked first if I'd have tested it correctly. At some point I have done either go get github.com/hybridgroup/gobot or a git clone https://github.com/hybridgroup/gobot ~/go/src/github.com/hybridgroup/. So I'd shadowed the gobot tree under ~/go/src/github.com/hybridgroup/gobot.

Then of course when I tested your PR I pulled your PR into the source tree rooted under ~/go/src/github.com/hybridgroup/gobot which is the wrong source tree and err, not even being built. I only realised this when I added some trace lines into you PR and they didn't appear.

go get aliasing might be useful in terms of keeping a project canonical path consistent but it does rely on a very fallible human, like me, to remember not to go get or git clone the root repository directly from github.

Apologies if I mislead you or Ron.

Owen

[owenwaller]

Hi Erkkah,

Nope we are still broken. Sorry. I can't write more than a single byte using WriteDataBlock. As soon as I change the code that writes the 18 byte data block needed to turn on the leds I get no output (and no errors). If I replace that call with a Write() that writes the 18 bytes I need for the leds + one more for the command, everything works again.

As far as I can tell there are two separate issues going on. One I can fix the second I think is down to you.

In the code the call to copy is wrong. You had:

copy(buf[:1], data)

But that needs to be:

copy(buf[1:], data)

Copy copies min(len(dest), len(src)) bytes. In your code buf[:1] is a slice is one byte. So you only copy one byte, not the whole array.

My current version - with some tracing and error detection - looks like this gist

As an example of what I am trying this output using the above gist should explain things:

Number of bytes copied: 18
len(data)=18
data:[]byte{0xff, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}
len(buf)=19
buf:[]byte{0x12, 0xff, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}

It looks like the call to d.smbusAccess(...) just isn't writing (all?) of the bytes. Your gist, includes an undefined constant I2C_SMBUS_BLOCK_DATA_PEC which might fix things but I need the value to try it. :)

Owen

[erkkah]

Hi Owen.

Great find that silly copy thing! Looks like we can share that humble pie :) And - that shadowing thing can be really confusing. I mainly use vendoring, and have fooled myself a number of times. Thank you so much for continuing to investigate.

My confusion, and what I would like to understand better, is the different "size" argument options. I have seen a bit of contradictory information, but now trust the "latest" linux header files (/usr/include/linux/i2c.h).

I believe the constants I have in https://github.com/erkkah/gobot/blob/i2c-pec-fix/sysfs/i2c_device.go, should work, and that the I2C_SMBUS_BLOCK_DATA (value 5) you use in your gist is correct.

However, the I2C lib that Toon's code is using uses I2C_SMBUS_I2C_BLOCK_DATA (value 8).

If it turns out that the latter is more widely supported, we should probably move to that, and change the functionality check accordingly.

Your latest gist, does it work?

/Erik

[erkkah]

BTW, that copy fix made both my devices "behave", accepting I2C_SMBUS_BLOCK_DATA writing just fine. Changing to I2C_SMBUS_I2C_BLOCK_DATA made one of the devices (TSL2561) fail.

/Erik

[owenwaller]

Ah ha! I have success.

So my change is:

I2C_SMBUS_BLOCK_DATA       = 8 //5

i.e. from 5 to 8. If I rebuild everying (with my copy fix) it works. I get a light on the PiGlow and no errors.

On my Pi a uname -r returns 4.4.38+

So yes, redefining I2C_SMBUS_BLOCK_DATA and using my gist means I have a working solution. But what you seem to be saying is that the value of I2C_SMBUS_BLOCK_DATA is kernel version dependant. Is that correct?

Owen

[erkkah]

Great news!

To answer your question - no, but I'll try to explain.

When entering this, I did what I use to do, and read the documentation. In this case, I went to https://www.kernel.org/doc/Documentation/i2c/dev-interface and https://www.kernel.org/doc/Documentation/i2c/smbus-protocol.

From there, I got the picture that I2C_SMBUS_BLOCK_DATA is to be preferred over I2C_SMBUS_I2C_BLOCK_DATA:

The following I2C block transactions are supported by the
SMBus layer and are described here for completeness.
They are *NOT* defined by the SMBus specification.

However, when I started to look into other I2C libraries, I found that they used the I2C_SMBUS_BLOCK_DATA_PEC constant (value 8) for block operations, which made me read up on PEC.

Now - the meaning of that constant seems to have changed with Linux versions, and PEC control is now a separate ioctl.

Don't know exactly when that changed, but it looks like a 2.X vs 4.X change. We should be safe with the values now in /usr/include/linux/i2c.h.

In addition to this confusion, different devices, as we now have shown, support different block operations. The PiGlow likes 8 but not 5, while the TSL2561 is the exact opposite. The DRV2605L supports both.

I'm starting to think that we should expose both, and document thoroughly.

@deadprogram, what do you think?

/Erik

[owenwaller]

You mean have a

WriteBlockData(...) and new WriteBlockI2CData(...)

We could make WriteBlockData(...) take a [32]byte not a []byte. That would both remove the if check, and fix the block length at a maximum. Which is what the protocol docs seem to suggest. That would be a breaking change though. WriteBlockI2CData(...) would still take a []byte. But then the protocol docs aren't clear. As I read this:

I2C block transactions do not limit the number of bytes transferred
but the SMBus layer places a limit of 32 bytes.

You can send more than 32 bytes to the IC, but things will be chunked into 32 byte blocks. So should the implementation do this chunking for the programmer? In this case the COMM "byte" can be multi-byte so presumably more than 32 bytes + whatever length of data it needs.

All of this would be fine with me. As a driver writer you'd then have to read the IC docs and take your best guess. If as I've found out, if one write call fails then you'd just have to try the other one.

Owen

[deadprogram]

This is a great conversation here.

Regarding block operations, that might also be platform/kernal version based, not device based. @owenwaller you are on Raspi but @erkkah you are using CHIP, correct? Some platforms still work on older kernels right now, I'd rather not lose that.

I'd also rather continue to let Driver authors avoid knowing quite this much about the low level i2c implementations if they do not have to. The original naive sysfs implementation has the advantage of not having to know as much about what you are doing to get it to work as you expect.

On a side not, we should probably failover to the sysfs equivalent operation if the desired block operation is not available, so Drivers can "just work", but can use the new helper functions.

The current WriteBlockData() should probably continue to accept []byte and then either return an error if over the limit, or automatically chunk the write. We should not make Driver authors have to deal with that themselves.

If for a particular Driver we need a specific block operation, and it is supported on the platform, we could use an optional config to set that, or something?

Anyhow those are a few of my thoughts. This is awesome.

[erkkah]

In my case the support is device dependent, but might very well be platform dependent as well. So, three dimensions of compatibility.. I'm starting to think that block operations are too complex to be worth supporting at this level. It seems as direct sysfs writes "always works" for block type writes.

Failover would be great, but in my case, would not work either. I never get any errors, the functionality checks seem to pass, yet it did not work.

If we keep WriteBlockData() I agree we should keep the []byte interface. Passing a [32]byte would need a separate length argument for all cases where the intention is to write less than 32 bytes.

Why I started to work on this area in the first place was that I could not get word size reads to work on the TSL2561 using the original implementation. Then I tried to make the interface as close to the underlying system calls as possible, reusing the same operation names, et.c.

I think it's easier now to write correct driver code using the exposed Byte and Word methods, but the Block stuff so far has proven the opposite.

So, right now, I would suggest removing block operations, or adding separate methods for the two kinds.

[deadprogram]

That probably makes sense, @erkkah

How about we leave in the WriteBlockData() interface, but just use it as a wrapper for a sysfs implementation for now. We can revisit the details on the "pure" SMBUS impl in the near future.

[owenwaller]

@deadprogram. Yes, I am on using a Raspi 1 running a 4.4.38+ kernel on jessie.

WriteBlockData should not chunk. It can only ever write 32 bytes max.. The SMBus spec seems to forbid chunking in this case. From the spec

The opposite of the Block Read command, this writes _up to_ 32 bytes to 
a device, to a designated register that is specified through the
Comm byte. The amount of data is specified in the Count byte.

If the data parameter remains as a []byte then it should error if its passed more than 32 bytes. The new WriteBlockI2CData that I'm suggesting has no max limit, but would instead write all the data out in 32 byte chunks automatically.

I would agree that the signature or WriteBlockData should not change. But nor should we delete/depreciate it in favour of just a Read/Write pair.

@erkkah one difference I have with the PiGlow is that its a write only device. You can't read the values of registers from the IC itself.

In terms of knowing which function to use. I've just checked the PiGlow's datasheet and it doesn't say anything about writing to the SMBus in I2C_SMBUS_BLOCK_DATA vs. I2C_SMBUS_I2C_BLOCK_DATA mode. To paraphrase the data-sheet it just says "write bytes to this address to get things to happen". So from the data-sheet alone I'd have no way to know which one of WriteBlockData or WriteBlockI2CData to call. I'd just have to try both and find which one works.

Having thought about this again is not ideal. It makes me question my suggestion of a new WriteBlockI2CData method in the first place.

But I don't see a way to avoid it. The functions act differently in relation to chunking - as per the spec.

The only other option that I see, would be to replace the implementation of WriteBlockData (and maybe all the WriteXXXX methods) with simple call the the underlying Write. @deadprogram is this in effect way you are saying? But then do you error if you are passed more than 32 bytes? I'm not sure you can...

Is the flip-side of this decision that we'd be using userland access to the i2c device. Rather than off loading the work to the kernel side driver as the native SMBus ioctl's would do?

I am not sure there are any easy or clear cut decisions on this, other than that we can't break existing code that is out in the wild.

Owen

[owenwaller]

@erkkah Thinking about this for 2 seconds more :).

Why don't you try the reverse of what I did. In your TSL2561 code, swap all of your ReadXXX calls for Read and similarly swap all of the WriteXXX calls for Write.

Does your device still work? Do you need to care if you write (or Read?) form the SMBus in I2C_SMBUS_BLOCK_DATA or I2C_SMBUS_I2C_BLOCK_DATA?

Owen

[erkkah]

Hi @owenwaller, the reason I started to look into this in the first place was that I could not get the TSL2561 to work using simple Read/Write operations. :)

I like the suggestion from @deadprogram, to back off the smbus block operations until they are really needed, but keeping the WriteBlockData() as a convenience. However, I think we should remove ReadBlockData() for now since that is not easily implemented using Read().

That would leave us with a less confusing, and more widely compatible I2C layer.

The WriteBlockData() implementation should still limit writes to 32 bytes. Don't think it will be a problem, since that is the expected behavior. If there is a need to write more, we are back in I2C (non-SMBus) land and plain Write() is the appropriate operation.

[deadprogram]

This seems like a good plan. What needs to be done next?

[erkkah]

I put together a version with the suggested changes in place, and added capabilities checks for all smbus operations: https://github.com/erkkah/gobot/commits/i2c-remove-smbusblock.

Had to extend MockSyscall to make tests pass, hope that was OK.

Take a look, if it seems to work, I can make it a PR.

[owenwaller]

I'm not sure. But I need to check my facts first :)

So for the PiGlow, I'm calling (d *i2cDevice) WriteBlockData(reg uint8, data []byte) (err error) in sysfs/i2c_device.go. And that needs to write to the SMBus in I2C_SMBUS_I2C_BLOCK_DATA mode not I2C_SMBUS_BLOCK_DATA modes which is what the code v1.2 code currently does.

But just to confuse things there we also have this: func (c *i2cConnection) WriteBlockData(reg uint8, b []byte) (err error) in drivers/i2c/ic2.go. Which as far as I know I'm not calling at all...

So my first dumb question is: Why do we have both of these? They appear to do the same things in different ways. Where are they different? Which one is the "SMBus" one and which one is the "ic2" one?

I actually have two bits of code at the minute. A really simple "light an led" program for the PiGlow. This is the code I'm be using to debug the (d *i2cDevice) WriteBlockData problems. I'm using GoBot with @erkkah's patch when I build this code. This is the PiGlow code I'm been discussing so far.

But I also have a much larger bit of code that I plan to turn iinto the PiGlow driver got GoBot. That code is working well but it replaces (d *i2cDevice) WriteBlockData with this:

unc (p *PiGlowDriver) writeBlockData(address byte, values []byte) error {
    b := []byte{address}
    b = append(b, values...)
    w, err := p.conn.Write(b)
    if err != nil {
        return err
    }
    if w != len(b) {
        return fmt.Errorf("failed to write the expected number of bytes. Wrote %d but expected %d", w, len(b))
    }
    return nil
}

where conn is an i2c.Connection. This code has been completely reliable.

So why don't we just replace: (d *i2cDevice) WriteBlockDatawith a similar Write() in a loop approach? This has the benefit of never needing to know which SMBus(?) write mode you should be in. If we want to clip this at 32 bytes then we can.

@erkkah at the minute I don't understand why ReadDataBlock (I assume from i2cDevice) can't be similarly implemented as a Read in a loop.

Thoughts?

Owen

[owenwaller]

@erkkah So looking at the PR you have more of less done what I suggested in WriteBlockData. So that's fine. I believe the Write() I am calling maps down to you d.file.Write(). But correct me if I am wrong.

But looking at the ReadBlockData calls that you have removed it does look like it can be reimplemented as a Read() in a loop.

Owen

[erkkah]

@owenwaller, the layer in drivers/i2c does two things, first it adds support for i2c on platforms that do not have sysfs, currently the firmata has it's own implementation. Second, it makes it possible to support several different i2c buses at the same time in a consistent manner.

In the sysfs case, it acts as a proxy on top of sysfs/i2c_device.go, so you are right that the interfaces are duplicated.

The general idea of the chosen set of methods is to make it easy to follow datasheets and preexisting example code. WriteBlockData happens to be part of the SMBus lingo, and is always limited to 32 bytes. For a pure SMBus device, it is not possible to use Write() with bigger blocks.

The challenge with implementing ReadBlockData() using Read() is to write the register/command byte to the device before reading. Some devices will accept setting the register/command by a single byte write before reading. If that is not accepted, Read() will always start from register 0, so even if you want to read from regs 5 to 15 you would have to read the first 5 and throw away the result.

Because of this, I think it is safe to leave ReadBlockData() out.

[owenwaller]

@erkkah. OK two different implementations to access i2c for platforms with different capabilities. That makes sense.

If I follow this correctly. Then a Raspi has both implementations available. Yes?

If I need to use I2C_SMBUS_I2C_BLOCK_DATA to do a block write to the PiGlow. That would suggest it's not an SMBus compliant device. SMBus isn’t mentioned in the data sheet for the IC either. Yes?

So, if sysfs exists on a platform, is it intended to support the full range of i2c or just the SMBus subset?

In terms of ReadBlockData() yes I see what you mean. I don't think its hard to implement. You'd always try the Write() to limit the data first. Then, depending on the failure of the write (or not) you'd know if you needed to throw away bytes from the following Read(). But it is yucky.

Owen

[erkkah]

Hey, @owenwaller, I'm moving forward with the PR then, it should resolve this issue. Regarding your questions, I really don't know :) There is a lot to read on the subject, though, both on the Linux side and from hardware manufacturers.

/E

[owenwaller]

@erkkah Yes the PR should be fine when I looked at it. Let me know when its merged in and where and I'll update things and try building my test code again.

Owen

[deadprogram]

This PR is now in the dev branch and will go out with the next release. I'll leave this open until it lands. Thanks everyone!

[deadprogram]

This code was just released with v1.3.0 thanks everyone!

[owenwaller]

Sorry I've been away from working on my PiGlow project for a few days.

But, just to say that I've pulled the v1.3.0 release, updated my code to use the i2c WriteBlockData and everything still works. Yeah. So, yes, it looks like this is fixed.

Thank you everyone

Owen