dotnet / iot

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

Mcp23s17 cannot read registers #1216

Closed z-eh closed 1 year ago

z-eh commented 3 years ago

Hi!

After I patched the SpiAdapter.Read() function, to make it work, I realized that I cannot read any information from the registers. Maybe I'm the fool, with my patch, but before the patch it throws ArgumentException, because the SpiAdapter.Read -> readBuffer and writeBuffer length not the same.

My patch in SpiAdapter.cs is at line 48:

From this:

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

To this:

Span<byte> readBuffer = stackalloc byte[Math.Max(writeBuffer.Length, buffer.Length)];

I try to achive reading with GpioController class and with the Mcp23s17 class itself.

Thanks, Zoltan

krwq commented 3 years ago

Hey @z-eh can you share the error you're getting after applying that fix?

I suspect you'll need to calculate the size of the buffer (Max(ReadBufferLength, WriteBufferLength)) first and then use it for both write and read buffer and not just for the read buffer. Any reason you have closed https://github.com/dotnet/iot/pull/1208 ?

z-eh commented 3 years ago

I closed it because I think this solution maybe the cause of my issue. When I make it work, then I made a new pull request with a working solution.

krwq commented 3 years ago

sounds good, let me know if you need any help

Ellerbach commented 3 years ago

[Triage] @z-eh did you manage to make it working? Does it need any patch inside the binding?

z-eh commented 3 years ago

I can not manage to make it working... It was a complete mess and I want to make sure to not brake anything else. When 1.3 Release came out in December, then I tried it and it just failed again. So I'm waiting new Raspberry Pi boards to arrive and new Mcp23s17 chips too. (+ a PiFace Digital I/O 2) I want to make sure that it is not a hardware related issue on my side. As I saw the code had "some" bugs in this area, so fingers crossed. But my free work time for this is very limited.

ricortegal commented 2 years ago

I have a similar issue. I could not read from piface GPIO port. I changed 41 line from

Span<byte> writeBuffer = stackalloc byte[]
                {
                    GetOpCode(_deviceAddress, isReadCommand: true),
                    registerAddress
                };

to

Span<byte> writeBuffer = stackalloc byte[]
                {
                    GetOpCode(_deviceAddress, isReadCommand: true),
                    registerAddress,
                    0
                };

otherwise cause this exception:

Unhandled exception. System.ArgumentException: Parameters 'writeBuffer' and 'readBuffer' must have the same length.
   at System.Device.Spi.UnixSpiDevice.TransferFullDuplex(ReadOnlySpan`1 writeBuffer, Span`1 readBuffer) in P:\PROGRAMACION\C#\iotnet\iot\src\System.Device.Gpio\System\Device\Spi\Devices\UnixSpiDevice.cs:line 249
   at Iot.Device.Mcp23xxx.Mcp23xxx.SpiAdapter.Read(Byte registerAddress, Span`1 buffer) in P:\PROGRAMACION\C#\iotnet\iot\src\devices\Mcp23xxx\SpiAdapter.cs:line 52
   at Iot.Device.Mcp23xxx.Mcp23xxx.InternalRead(Register register, Span`1 buffer, Port port) in P:\PROGRAMACION\C#\iotnet\iot\src\devices\Mcp23xxx\Mcp23xxx.cs:line 125
   at Iot.Device.Mcp23xxx.Mcp23xxx.InternalReadByte(Register register, Port port) in P:\PROGRAMACION\C#\iotnet\iot\src\devices\Mcp23xxx\Mcp23xxx.cs:line 157
   at Iot.Device.Mcp23xxx.Mcp23x1x.ReadByte(Register register, Port port) in P:\PROGRAMACION\C#\iotnet\iot\src\devices\Mcp23xxx\Mcp23x1x.cs:line 36
   at Copermatica.Devices.PiFaceDigital2.ToggleRele1() in P:\PROGRAMACION\C#\iotnet\Piface.App\PiFaceDigital2.cs:line 96   at Piface.App.Program.Main(String[] args) in P:\PROGRAMACION\C#\iotnet\Piface.App\Program.cs:line 30
   at Piface.App.Program.<Main>(String[] args)

with the change works perfectly. I Can read GPIOA

I'm using a raspberry pi 3b with a piface2 hat, on raspbian OS with dotnet core 5.0.400 runtime.

pgrawehr commented 2 years ago

@ricortegal Which file is this change in?

ricortegal commented 2 years ago

SpiAdapter.cs:line 52

if the buffer has different length in

System.Device.Spi.UnixSpiDevice.TransferFullDuplex(ReadOnlySpan`1 writeBuffer, Span`1 readBuffer)

throws an exception. the real reason why it works, with the change, I don't know

ricortegal commented 2 years ago

@pgrawehr https://github.com/dotnet/iot/blob/0d68f522dc2b96fd7d598ae8acb0ca636200ebb2/src/devices/Mcp23xxx/SpiAdapter.cs#L41

pgrawehr commented 2 years ago

I'll take a look. Looks like a simple bug.

ghost commented 1 year ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.