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

Mcp23017 initialization resets pin values #1663

Open dme-development opened 3 years ago

dme-development commented 3 years ago

Describe the bug

Every time a new instance of Mcp23017() is being created the pins are being reset to low. This is an issue when the written software reboots, eg when updating the software version.

Steps to reproduce

            var pinNumber = 0;

            var i2cConnectionSettings = new I2cConnectionSettings(1, 0x20);

            var i2cDevice = I2cDevice.Create(i2cConnectionSettings);

            var device = new Mcp23017(i2cDevice);

            var controller = new GpioController(PinNumberingScheme.Logical, Mcp23017Device);

            controller.Write(pinNumber, PinValue.High);

            var device2 = new Mcp23017(i2cDevice);

            var pinValue = controller.Read(pinNumber);

Expected behavior

Pins need to keep their value after re-initializing the Mcp23017() adapter.

Actual behavior

Pins are being reset every time a new instance of Mcp23017() is being declared. Looks like it is embedded in the init code here: https://github.com/dotnet/iot/blob/main/src/devices/Mcp23xxx/Mcp23xxx.cs - starting at line 84.

pgrawehr commented 3 years ago

Thanks for reporting. This is a similar problem than what was fixed in #1520 for the main GPIO controllers.

@dme-development Do I see it correctly that your use case is that you drive some external hardware (relays or similar) using the Mcp23017 and you want to be able to update/restart the software without these changing?

There's a possible workaround: The state is not primarily reset to low, it is actually set to input. If you use a pull-up or pull-down you could at least override the default value (e.g. to prevent that the power of the device itself is killed on a reset).

dme-development commented 3 years ago

Do I see it correctly that your use case is that you drive some external hardware (relays or similar) using the Mcp23017 and you want to be able to update/restart the software without these changing?

@pgrawehr That is exactly our use case: when updating a docker image version, we want the relays to keep their values.

We have created our own PCB, pull-up/pull-down is currently not an option, but we created a workaround by removing the reset functionality from the library. Of course, we would like to see it being resolved in the library.

I think there have been similar issues for other device bindings, eg: #1540

pgrawehr commented 3 years ago

Yes, this is an use case that has been forgotten in the initial design of several APIs (see also my links above). You are welcome to submit a PR, but try not to break existing APIs (that means that you probably have to add an extra bool argument to the ctor).

krwq commented 2 years ago

[Triage] we need to fix initialization similarly to what we did here: https://github.com/dotnet/iot/issues/1479