Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GrovePi binding timing issue when read sensors #2359

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix GrovePi binding timing issue when read sensors #2359

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
45887bd
add retries when grovepi returns invalid response
2427dkusiro Oct 9, 2024
86db9a0
increase retry limit
2427dkusiro Oct 10, 2024
8d258e4
fix DigitalRead method to call common ReadCommand method
2427dkusiro Oct 10, 2024
93c6303
remove exception handling in DigitalRead method
Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 18 additions & 22 deletions src/devices/GrovePi/GrovePi.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Device.I2c;
using System.IO;
using System.Threading;

using Iot.Device.GrovePiDevice.Models;

namespace Iot.Device.GrovePiDevice
Expand All @@ -16,7 +17,7 @@ namespace Iot.Device.GrovePiDevice
/// </summary>
public class GrovePi : IDisposable
{
private const byte MaxRetries = 4;
private const byte MaxRetries = 10;
private readonly bool _shouldDispose;
private I2cDevice _i2cDevice;

Expand Down Expand Up @@ -100,7 +101,7 @@ public void WriteCommand(GrovePiCommand command, GrovePort pin, byte param1, byt
return;
}
catch (IOException ex)
{
{
innerEx = ex;
tries++;
Thread.Sleep(10);
Expand All @@ -118,9 +119,10 @@ public void WriteCommand(GrovePiCommand command, GrovePort pin, byte param1, byt
/// <returns></returns>
public byte[]? ReadCommand(GrovePiCommand command, GrovePort pin)
{
const int dataNotAvailableCommand = 23;
int numberBytesToRead = command switch
{
GrovePiCommand.DigitalRead => 1,
GrovePiCommand.DigitalRead => 2,
GrovePiCommand.AnalogRead or GrovePiCommand.UltrasonicRead or GrovePiCommand.LetBarGet => 3,
GrovePiCommand.Version => 4,
GrovePiCommand.DhtTemp => 9,
Expand All @@ -143,15 +145,23 @@ public void WriteCommand(GrovePiCommand command, GrovePort pin, byte param1, byt
try
{
_i2cDevice.Read(outArray);
return outArray;
}
catch (IOException ex)
{
// Give it another try
innerEx = ex;
tries++;
Thread.Sleep(10);
continue;
}

if (outArray[0] != dataNotAvailableCommand && outArray[0] != 255)
{
return outArray;
}

tries++;
Thread.Sleep(10);
}

throw new IOException($"{nameof(ReadCommand)}: Failed to write command {command}", innerEx);
Expand All @@ -165,27 +175,13 @@ public void WriteCommand(GrovePiCommand command, GrovePort pin, byte param1, byt
public PinValue DigitalRead(GrovePort pin)
{
WriteCommand(GrovePiCommand.DigitalRead, pin, 0, 0);
byte tries = 0;
IOException? innerEx = null;
// When writing/reading to the I2C port, GrovePi doesn't respond on time in some cases
// So we wait a little bit before retrying
// In most cases, the I2C read/write can go thru without waiting
while (tries < MaxRetries)
var data = ReadCommand(GrovePiCommand.DigitalRead, pin);
if (data is null || data.Length < 2)
{
try
{
return (PinValue)_i2cDevice.ReadByte();
}
catch (IOException ex)
{
// Give it another try
innerEx = ex;
tries++;
Thread.Sleep(10);
}
return (PinValue)(-1);
}

throw new IOException($"{nameof(DigitalRead)}: Failed to read byte with command {GrovePiCommand.DigitalRead}", innerEx);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removing the throw here and replace with the -1 logic? Throwing is a good way to say that things did not work properly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ellerbach
I initially considered making this change because the existing AnalogRead method was designed to return -1 instead of throwing an exception. However, I have adjusted the behavior at commit 93c6303 accordingly.

return (PinValue)data[1];
}

/// <summary>
Expand Down
Loading