dotnet / iot

This repo includes .NET Core implementations for various IoT boards, chips, displays and PCBs.
MIT License
2.18k stars 586 forks source link

System.ArgumentException: Parameters 'writeBuffer' and 'readBuffer' must have the same length #917

Open olavt opened 4 years ago

olavt commented 4 years ago

I wrote a small test program and when running it on a Raspberry PI 3 with a PiFace Digital 2, I get the following exception:

Unhandled exception. System.ArgumentException: Parameters 'writeBuffer' and 'readBuffer' must have the same length. at System.Device.Spi.UnixSpiDevice.TransferFullDuplex(ReadOnlySpan1 writeBuffer, Span1 readBuffer) at Iot.Device.Mcp23xxx.Mcp23xxx.SpiAdapter.Read(Byte registerAddress, Span1 buffer) at Iot.Device.Mcp23xxx.Mcp23xxx.InternalRead(Register register, Span1 buffer, Port port) at Iot.Device.Mcp23xxx.Mcp23xxx.InternalReadByte(Register register, Port port) at Iot.Device.Mcp23xxx.Mcp23x1x.ReadByte(Register register, Port port) at TestPiface.Program.Main(String[] args) in /home/vsts/work/1/s/Electronics/Implementations/TestPiface/Program.cs:line 31

The line causing the issue is:

        byte currentValue = mcp23s17.ReadByte(Register.GPIO, Port.PortA);

Shouldn't it be possible to read the state of the output ports like this?

This is the complete program:

using System; using System.Threading.Tasks; using System.Threading; using Iot.Device.Mcp23xxx; using System.Device.Spi;

namespace TestPiface { class Program { private static readonly int s_deviceAddress = 0x20;

    private static Mcp23s17 mcp23s17;

    static void Main(string[] args)
    {
        var spiConnectionSettings = new SpiConnectionSettings(0, 0)
        {
            ClockFrequency = 1000000,
            Mode = SpiMode.Mode0
        };

        var spiDevice = SpiDevice.Create(spiConnectionSettings);

        mcp23s17 = new Mcp23s17(spiDevice, s_deviceAddress);

        mcp23s17.WriteByte(Register.IODIR, 0b0000_0000, Port.PortA);    // set port A as an output
        mcp23s17.WriteByte(Register.IODIR, 0b1111_1111, Port.PortB);    // set port B as an input
        mcp23s17.WriteByte(Register.GPIO, 0b0000_0000, Port.PortA);     // Clear all outputs

        byte currentValue = mcp23s17.ReadByte(Register.GPIO, Port.PortA);

        mcp23s17.WriteByte(Register.GPIO, 0b0000_0010, Port.PortA);
    }

}

}

krwq commented 4 years ago

cc: @shaggygi

shaggygi commented 4 years ago

@krwq I honestly have not used this device with the new BusAdapter scheme.

I'm thinking it blows up in this method call as the buffer sizes are updated before calling TransferFullDuplex.

Briefly reviewing the code, it looks like the buffer is set to a size of 1 before calling the Read method. writeBuffer is set to a size of 2 with GetOpCode and registerAddress (line 42). readBuffer is set to a size of 3 on line 48).

krwq commented 4 years ago

thanks, no problem @shaggygi I'll see if I have some time to take a look at this (I think I have this expander lying around somewhere)

gsutra commented 4 years ago

Hi,

We are facing the same issue. @krwq Would you have a quick workaround ?

joperezr commented 4 years ago

@gsutra do you mind sharing the callstack and if possible your sample repro code as well so we can investigate and either fix the issue or give you a workaround?

lpayet commented 4 years ago

Hi, I'm working with @gsutra.

Here is the code : ` using Iot.Device.Mcp23xxx; using System; using System.Device.Gpio; using System.Device.Spi;

namespace Test { public class Program { public static void Main(string[] args) { var spiConnectionSettings = new SpiConnectionSettings(2, 1) { ClockFrequency = 1000000, Mode = SpiMode.Mode0 }; SpiDevice spiDevice = SpiDevice.Create(spiConnectionSettings);

        Mcp23s17 mcp23S17 = new Mcp23s17(spiDevice, 0x20);

        byte result = mcp23S17.ReadByte(Register.GPIO, Port.PortA);
        Console.WriteLine(result);
    }
}

} `

And the callstack :

Unhandled exception. System.ArgumentException: Parameters 'writeBuffer' and 'readBuffer' must have the same length. at System.Device.Spi.UnixSpiDevice.TransferFullDuplex(ReadOnlySpan1 writeBuffer, Span1 readBuffer) at Iot.Device.Mcp23xxx.Mcp23xxx.SpiAdapter.Read(Byte registerAddress, Span1 buffer) at Iot.Device.Mcp23xxx.Mcp23xxx.InternalRead(Register register, Span1 buffer, Port port) at Iot.Device.Mcp23xxx.Mcp23xxx.InternalReadByte(Register register, Port port) at Iot.Device.Mcp23xxx.Mcp23x1x.ReadByte(Register register, Port port) at Test.Program.Main(String[] args) in Test\Program.cs:line 21 Abandon

fennixsystems commented 4 years ago

Was a solution ever found to this issue? I'm experiencing it with the latest build and very similar/basic test code.

shaggygi commented 4 years ago

@fennixsystems I still believe the error is within my comment above, but don't believe there was a fix yet.

fennixsystems commented 4 years ago

@shaggygi Any idea when there might be a fix? Or even a fix I can implement myself temporarily?

SaDiablo commented 4 years ago

This might be caused by improper usage of transfer full duplex in Mcp23xxx\SpiAdapter.cs

public override void Read(byte registerAddress, Span<byte> buffer)
{
    // Include OpCode and Register Address.
    Span<byte> writeBuffer = stackalloc byte[]
    {
        GetOpCode(_deviceAddress, isReadCommand: true),
        registerAddress
    };

    Span<byte> readBuffer = stackalloc byte[buffer.Length + 2];

    // Should this also contain the op code and register?
    // Why are we transferring full duplex if we only really
    // need to read?
    _device.TransferFullDuplex(writeBuffer, readBuffer);

    // First 2 bytes are from sending OpCode and Register Address.
    readBuffer.Slice(2).CopyTo(buffer);
}

^ If you are able you might check what sizes are readBuffer and writeBuffer in your case and try to see what's going on. (this might help you debug your RPi code remotely: Debugging Your .Net Core 3 App on Your Raspberry Pi with Visual Studio 2019)

As a one way to you can try to fix it is to use software version of spi, because this exact error is impossible to occur (error in the code shown below) but it probably won't work in further steps or cause more bugs

SoftwareSpi.cs

if (dataToRead.Length != dataToRead.Length)
{
    throw new ArgumentException(nameof(dataToWrite));
}

whereas Unix and Windows spi Devices

if (writeBuffer.Length != readBuffer.Length)
{
    throw new ArgumentException($"Parameters '{nameof(writeBuffer)}' and '{nameof(readBuffer)}' must have the same length.");
}

so that's something to take note of.

Hope that helps you at least a bit

joperezr commented 4 years ago

Thanks for the investigation @SaDiablo. Marked this as vNext and up-for-grabs if anybody wants to fix this on the MCP binding.

nick-mitigo commented 3 years ago

I faced this issue with RPI4 and the old PiFaceV1. This board is using the Mcp23s17 chip. The biggest challenge was to get the IOT lib running and allow debugging - finally it worked through VSCode. My current fix is just sending one more byte during the read operation:

                // Include OpCode and Register Address.
                Span<byte> writeBuffer = stackalloc byte[]
                {
                    GetOpCode(_deviceAddress, isReadCommand: true),
                    registerAddress,
                    0 // this solved the problem
                };

I don't really understand the current addressing of the chip, why is the 0x20-0x27 address range used? The chip has only 3 address bits, so for me, it would more natural to select them by specifying the range 000 - 111 (0-7). This is the opcode structure:

                    +--------------------+
                    |0|1|0|0|A2|A1|A0|R/W|
                    +--------------------+
                     7 6 5 4 3  2  1   0

I've changed the GetOpCode according to this - however, maybe I'm not seeing the big picture, I'm not a professional embedded guy.. :) I have a small PiFaceV1 wrapper class - would it be helpful to add to the IOT lib?

Ellerbach commented 3 years ago

I have a small PiFaceV1 wrapper class - would it be helpful to add to the IOT lib?

Hi! Quite late to answer, was checking some issues, and to answer, yes, any PR is appreciated! Thanks!