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.

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 bug where SHTC3 was not woken from sleep before operations #2034

Merged
merged 2 commits into from Feb 10, 2023

Conversation

jcoliz
Copy link
Contributor

@jcoliz jcoliz commented Feb 10, 2023

As discussed in Discord.

SHTC3 binding would only ever read one set of values, before triggering this exception:

Unhandled exception. System.IO.IOException: Error 121 performing I2C data transfer.
   at System.Device.I2c.UnixI2cBus.WriteReadCore(UInt16 deviceAddress, Byte* writeBuffer, Byte* readBuffer, UInt16 writeBufferLength, UInt16 readBufferLength) in C:\Source\jcoliz\dotnet\iot\src\System.Device.Gpio\System\Device\I2c\UnixI2cBus.cs:line 197
   at System.Device.I2c.UnixI2cBus.Write(Int32 deviceAddress, ReadOnlySpan`1 buffer) in C:\Source\jcoliz\dotnet\iot\src\System.Device.Gpio\System\Device\I2c\UnixI2cBus.cs:line 125
   at System.Device.I2c.UnixI2cDevice.Write(ReadOnlySpan`1 buffer) in C:\Source\jcoliz\dotnet\iot\src\System.Device.Gpio\System\Device\I2c\Devices\UnixI2cDevice.cs:line 28
   at Iot.Device.Shtc3.Shtc3.Write(Register register) in C:\Source\jcoliz\dotnet\iot\src\devices\Shtc3\Shtc3.cs:line 247
   at Iot.Device.Shtc3.Shtc3.TryReadSensorData(Register cmd, Double& temperature, Double& humidity) in C:\Source\jcoliz\dotnet\iot\src\devices\Shtc3\Shtc3.cs:line 148
   at Iot.Device.Shtc3.Shtc3.TryGetTemperatureAndHumidity(Temperature& temperature, RelativeHumidity& relativeHumidity, Boolean lowPower, Boolean clockStretching) in C:\Source\jcoliz\dotnet\iot\src\devices\Shtc3\Shtc3.cs:line 133

That's because the device was put to sleep (in the sample), but never actually woken up before getting the next reading. THAT was because the status was never set when device put to sleep.

Microsoft Reviewers: Open in CodeFlow

@ghost ghost added the area-device-bindings Device Bindings for audio, sensor, motor, and display hardware that can used with System.Device.Gpio label Feb 10, 2023
Copy link
Member

@Ellerbach Ellerbach left a comment

Choose a reason for hiding this comment

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

Thanks for looking at this. Can you as well update the example and wake up the sensor? And then also update the README?

Write(Register.SHTC3_WAKEUP);

_status = Status.Idle;

Copy link
Member

Choose a reason for hiding this comment

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

Just remove this extra line

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops! Sorry about that. Updated now.

@jcoliz
Copy link
Contributor Author

jcoliz commented Feb 10, 2023

@Ellerbach, thanks for reviewing. Doesn't look like the sample nor README need change. What did you have in mind?

The sample doesn't need to explicitly wake the sensor, because the call to TryGetTemperatureAndHumidity() does so implicitly. With this PR, that implicit wakeup will actually happen as-designed.

Likewise, the sample isn't able to explicitly wake the sensor, because WakeUp() is private. Not trying to be pedantic here... It seems that the design of the binding is that user explicitly sleeps the sensor when done with it, and then the binding will implicitly wake it back up when user asks for an operation. This seems like a reasonable design, so I didn't see any need to suggest making the user explicitly wake the sensor when they're ready to use it.

Copy link
Member

@krwq krwq left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@krwq krwq merged commit 3ff5e60 into dotnet:main Feb 10, 2023
@jcoliz jcoliz deleted the pull/shtc3/1 branch February 10, 2023 22:25
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-device-bindings Device Bindings for audio, sensor, motor, and display hardware that can used with System.Device.Gpio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants