robsonj / GrovePi

Windows 10 IoT C# driver library for GrovePi
17 stars 10 forks source link

Fixed TemperatureAndHumiditySensor #22

Closed maxpg closed 8 years ago

maxpg commented 8 years ago

Not sure about the state of this sensor but it didn't work for the TemperatureAndHumidity Sensor Pro.

Adapted sensor to work with the TemperatureAndHumidity Sensor Pro. Delivers temperature and humidity with a single call to TemperatureAndHumidity().

For the pro sensor make sure to initialize the sensor with model ONE

benoitjadinon commented 8 years ago

hi @maxpg , where you part of the #EmHackinathon this week end ? Cause I also noticed an issue yesterday with the temperature sensor code and wanted to fix it. But I've documented a bit and found out there is also a temperature sensor that does only temperature, so the code you've modified probably still works with those sensors, what we need is a new class for sensors that do both (there are 3 of them)

Temperature Sensor v1.0 http://www.seeedstudio.com/wiki/Grove_-_Temperature_Sensor

Temperature Sensor v1.1 v1.2 http://www.seeedstudio.com/wiki/Grove_-_Temperature_Sensor_V1.2

Temperature And Humindity Sensor DHT11 http://www.seeedstudio.com/wiki/Grove_-_Temperature_and_Humidity_Sensor

Temperature And Humindity Sensor HDC1000 http://www.seeedstudio.com/wiki/Grove_-_Temperature%26Humidity_Sensor_(HDC1000)

Temperature And Humindity Sensor TH02 http://www.seeedstudio.com/wiki/Grove_-_Tempture%26Humidity_Sensor_(High-Accuracy_%26Mini)_v1.0

maxpg commented 8 years ago

Hi benoitjadinon,

no, I was not. Ah, did not realize that the sensor is for the temperature sensors as it was called TemperatureAndHumidity. So I thought it should handle the sensors that deliver temperature and humidity.

This explains my confusion why in the existing code uses a read from the analog input while the DHT sensors are digital.

If someone hints me in a direction where the code should evolve to (one sensor that handles all these models or one sensor for the analog temperature sensors and one for the DHT sensors) I can adapt the change in this direction.

maxpg

benoitjadinon commented 8 years ago

hi ok cool, coincidence then, I think you could get the old class back (and its enums, they correspond to the sensor versions => 1.2 : OnePointTwo), but maybe rename it to something like TemperatureOnly. sadly I don't have a temperature sensor so I can't test, but I'm guessing it had been tested when only those sensors were available. Then your changes would fit the TemperatureAndHumidity class, but it may need an enum for the sensor type (DHT11, HDC1000, TH02), cause I think the connection is different from one sensor to the other. you probably have the DHT like me. the enum Model only fits the TemperatureOnly class. thanks!

benoitjadinon commented 8 years ago

there seem to be (different) version numbers for the DHT sensor as well : https://github.com/Seeed-Studio/Grove_Temperature_And_Humidity_Sensor/blob/master/DHT.h

benoitjadinon commented 8 years ago

each sensor has a different way to connect, so they would need to be in separate classes

robsonj commented 8 years ago

I don’t have a windows machine at my disposal right now, but if you make the appropriate changes, I’ll merge them into the trunk. I think just fixing the TemperatureAndHumidity is the right thing to do.

Cheers Jonathan

On Oct 19, 2015, at 7:01 AM, Benoit Jadinon notifications@github.com wrote:

hi ok cool, coincidence then, I think you could get the old class back (and its enums, they correspond to the sensor versions => 1.2 : OnePointTwo), but maybe rename it to something like TemperatureOnly. sadly I don't have a temperature sensor so I can't test, but I'm guessing it had been tested when there was only those sensors available. Then your changes would fit the TemperatureAndHumidity class, but it may needs an enum for the sensor type (DHT11, HDC1000, TH02), cause I think the connection is different from one sensor to the other. you probably have the DHT like me. thanks!

— Reply to this email directly or view it on GitHub https://github.com/robsonj/GrovePi/pull/22#issuecomment-149195453.

maxpg commented 8 years ago

I updated my code as follows:

Moved the existing implementation from class TemperatureAndHumiditySensor to class TemperatureSensor. It contains the support for the temperature sensors (without humidity) in versions 1.0, 1.1 and 1.2. So existing users of the sensors would have to use the TemperatureSensor in the future.

Adapted the class TemperatureAndHumiditySensor to support the two sensors Temperature and Humidity Sensor (DHT11) and Temperature and Humidity Sensor Pro (DHT22).

There are also other Temperature and Humidity Sensors like the Temperature and Humidity Sensor (High-accuracy & mini) 1.0 which is currently not supported as I don't have one for testing.

benoitjadinon commented 8 years ago

working great with my DHT11, thanks !

PerryVanDerMeeren commented 8 years ago

Works for me too (DHT11). Thank you very much Max.

amitary commented 8 years ago

The fix is still not merged into the GrovePi nuget package? If not then expected by when?