GrovePi device binding fails to read sensor values

[2427dkusiro]

Describe the bug

When using the DigitalRead and AnalogRead functions included in the device binding for GrovePi to read sensor values, the functions return -1 instead of the sensor output, making it impossible to read sensor values.

Using the Python library provided by the device manufacturer, correct values can be read.

When reproducing the issue using AnalogRead, the binary data obtained from I2CDevice.Read is 23 255 255, and in the Python binding, 23 is declared as data_not_available_cmd which triggers a retry condition.

In dotnet, by modifying the code to perform a retry when the first byte of the response is 23, correct sensor values can be read.

If this issue and the proposed fix are deemed appropriate, I would like to submit a PR.

Steps to reproduce

1. Connect a Raspberry Pi model 4B with the GrovePi board and a push-button module.
2. Execute the following minimal C# code, based on the sample code:
3. The values written to the standard output will be -1 regardless of the button operation.

I2cConnectionSettings i2CConnectionSettings = new(1, GrovePi.DefaultI2cAddress);
using GrovePi grovePi = new GrovePi(I2cDevice.Create(i2CConnectionSettings));

Console.WriteLine($"Manufacturer: {grovePi.GrovePiInfo.Manufacturer}");
Console.WriteLine($"Board: {grovePi.GrovePiInfo.Board}");
Console.WriteLine($"Firmware version: {grovePi.GrovePiInfo.SoftwareVersion}");

var buttonPin = GrovePort.AnalogPin0;
grovePi.PinMode(buttonPin, PinMode.Input);

for (int i = 0; ; i++)
{
    var value = grovePi.AnalogRead(buttonPin);
    Console.WriteLine($"{i}: button={value}");

    Thread.Sleep(100);
}

Expected behavior

The AnalogRead function should return the sensor values. internal I2CDevice.Read function should return responses like 3 3 255 (indicating maximum voltage level) or 3 0 0 (indicating minimum voltage level).

Actual behavior

The AnalogRead function returns -1. The internal I2CDevice.Read function returns a response of 23 255 255 .

Versions used

Compiled and executed on dotnet8 running on a Raspberry Pi 4. The GrovePi board firmware used is the latest version 1.4.

The library version used System.Device.Gpio 3.2.0 and Iot.Device.Bindings 3.2.0 though other major releases were also tested with the same result.

.NET SDK:
 Version:           8.0.303
 Commit:            29ab8e3268
 Workload version:  8.0.300-manifests.c915c39d
 MSBuild version:   17.10.4+10fbfbf2e

Runtime Environment:
 OS Name:     debian
 OS Version:  11
 OS Platform: Linux
 RID:         linux-arm64
 Base Path:   /home/pi/.dotnet/sdk/8.0.303/

.NET workloads installed:
There are no installed workloads to display.

Host:
  Version:      8.0.7
  Architecture: arm64
  Commit:       2aade6beb0

.NET SDKs installed:
  8.0.303 [/home/pi/.dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 8.0.7 [/home/pi/.dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 8.0.7 [/home/pi/.dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  DOTNET_ROOT       [/home/pi/.dotnet]

global.json file:
  Not found

[pgrawehr]

Does this always return -1 or only the first time? If so, why would a retry help?

[2427dkusiro]

When executing the reading consecutively, most readings return -1, and occasionally they return what seems to be correct values.
example: -1 -1 -1 ... -1 1023 1021 1023 -1 -1 ...

This is likely because the time lag between sending the read command from the host machine to the MCU that actually reads the sensor values and the sensor values becoming available is not being accounted for.

[krwq]

[Triage] This seems like a timing issue - this was tested on Raspberry Pi 3 and likely 4 is faster and timing is slightly off (possibly we need to add delay somewhere where it wasn't needed before). @2427dkusiro are you interested in debugging and sending a patch? You will need to likely edit https://github.com/dotnet/iot/blob/main/src/devices/GrovePi/Sensors/AnalogSensor.cs

[2427dkusiro]

I am currently working on an IoT project using this board, and I have already implemented a sample workaround that is working. After a bit more functional testing and some tweaks, I can submit it as a PR.