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

DHT Temperature and Humidity plus Heat Index (feels like temperature) - using official Adafruit Library #35

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

cnerone
Copy link

@cnerone cnerone commented Dec 27, 2018

I rewrote this example,
it runs good and it uses Adafruit official DHTU libraries; furthermore I included Heat Index computation(based on DHT adafruit library) so this examples supply 3 sensors data (humidity, temperature, heat index)

@cnerone
Copy link
Author

cnerone commented Dec 27, 2019

can anyone help me to get it published?
I think it's an improvement and it runs very good...

#define DHTPOWERPIN 8


// Sleep time between sensor updates (in milliseconds) to add to sensor delay (read from sensor data; tipically: 1s)
Copy link

Choose a reason for hiding this comment

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

Typo: typically

Copy link
Author

Choose a reason for hiding this comment

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

I'm going to correct it.
thank you

present(CHILD_ID_HUM, S_HUM);
present(CHILD_ID_TEMP, S_TEMP);

present(CHILD_ID_HUM, S_HUM, "Umidity");
Copy link

Choose a reason for hiding this comment

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

Typo: "Humidity"

Copy link
Author

Choose a reason for hiding this comment

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

same answer than before:
I'm going to correct it.
thank you


MyMessage msgHum(CHILD_ID_HUM, V_HUM);
MyMessage msgTemp(CHILD_ID_TEMP, V_TEMP);
DHT dht;
MyMessage msgTempHI(CHILD_ID_TEMP_HI, V_TEMP);
Copy link

@mvdw mvdw Jan 1, 2020

Choose a reason for hiding this comment

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

a better name would be "msgHeatIndex", CHILD_ID_HEATINDEX, SENSOR_HEATINDEX_OFFSET

Copy link
Author

Choose a reason for hiding this comment

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

I appreciate your suggestion.
I'm going to change it

@cnerone
Copy link
Author

cnerone commented Jan 1, 2020

can I "accept" your revision editing file here (by browser in github) or I have to upload a new release by same process I used one year ago?

thanks in advance. have a nice year

@mvdw
Copy link

mvdw commented Jan 4, 2020

You're welcome.
I don't know, I do not mind.

@cnerone cnerone changed the title DHT - Using official Adafruit Library and Heat Index (feels like sensor) DHT Temperature and Humidity plus Heat Index (feels like sensor) - using official Adafruit Library Jan 4, 2020
@cnerone cnerone changed the title DHT Temperature and Humidity plus Heat Index (feels like sensor) - using official Adafruit Library DHT Temperature and Humidity plus Heat Index (feels like temperature) - using official Adafruit Library Jan 4, 2020
@mfalkvidd
Copy link
Member

mfalkvidd commented Jan 5, 2020

Thanks to both of you. Sorry for not giving feedback earlier.

As mentioned in #22 we would like to keep the dht example as simple as possible. A lot of code might scare people who are new to MySensors, and the dht sensor is (unfortunately) often one of the first sensors people try. Maybe we should have one "bare minimum" example and one more advanced?

For many of the other third party libraries, we've copied the entire libraryinto this repo. The idea (I think) is to make sure we provide a version that is guaranteed to be compatible with this sketch, and save the user from the hassle of having to download the upstream library.

This strategy hasn't worked that well though. A lot of people install the upstream version anyway, which leads to problems (warning due to multiple installations, compilation errors when the interfeces have changed, etc). I am not sure how to handle third part libraries in a good way. Maybe the sketch should include a comment on what people should search for in the Arduino IDE Library manager (library name) and which version(s) the sketch has been tested with?

This is to large and complex sketch for newbie to overwrite DHT humidity sensor one, Mikael Falkvidd said and I agree.
So I suggest to upload it as a different, more sophisticated example; indeed it could be incorrect to consider it as a simple Humidity sensor...
@cnerone
Copy link
Author

cnerone commented Jan 6, 2020

@mfalkvidd
Hi,
I understand, I ask you to delete changes I proposed for original file (DhtTemperatureAndHumiditySensor.ino), I'm not able to remove my modifications, simply reject my suggestions.
Meanwhile I posted my code in a different file (TempHumFeel.ino); so we have simple way and complex one! I hope this is a good way!
I suggest to create a dedicated page on mysensors.org or to add a note to http://www.mysensors.org/build/humidity linking to the sophisticated way....

@mfalkvidd
Copy link
Member

mfalkvidd commented Jan 6, 2020

Sounds good.

I think the right method is to do git rebase -i, remove all unnecessary commits and then git push —force. See if you can do that. Otherwise I’ll try to do it as soon as I’m close to a computer.

@cnerone
Copy link
Author

cnerone commented Jan 6, 2020

@mfalkvidd
I suppose I'll be not able to do that in next days because of job and family...
If you'll do it I'm very glad to you

Thanks for your efforts and gentleness... and sorry for my poor english..

@mfalkvidd
Copy link
Member

No worries, I'll try to do it tomorrow.

@mfalkvidd
Copy link
Member

mfalkvidd commented Jan 8, 2020

Sorry, I am unable to figure out how to do git rebase -i on someone else's fork.

@ericvb
Copy link

ericvb commented Apr 26, 2020

What is the status of this pull request?

To my opinion, copying external libraries into this repo, for the sake of being sure that the sketch will be 'working', is not a very good idea.
With the time passing, libraries will be updated (bug updates, security updates, feature requests, …) or abandoned :-(. With the latter, you could argue that it is indeed better that the library is copied in the repo. But then I would update the sketch to use another library or if it can't be done, delete the example. (for example a sketch using a deprecated hardware component)

Breaking changes in updated libraries could be easily detected by including the build of the sketches into the nightly builds of Jenkins (I thought have been reading that MySensors was using Jenkins to build?)

I think also that people starting with MySensors, do have already experience with using Arduino's and the Arduino Editor with his libraries manager.
Putting inside the sketch the official names of the libraries is indeed a very good idea.
I compiled the sketch of cnerone without any problems after installing the mentioned libraries.

@mfalkvidd, we communicated already about this same issue for the Distance Sensor example. This example is using an old NewPing library version...
The current given example is not working in 'a real world use of the sensor' because of the '0' distance output of the library in case of an error.

Just my two cents opinion :-)

@ericvb
Copy link

ericvb commented Apr 26, 2020

@cnerone, I analysed your sketch and I think there are some logical mistakes.
You are providing the possibility to give offsets
#define SENSOR_HUM_OFFSET 0 // used for temperature data and heat index computation
#define SENSOR_TEMP_OFFSET 0 // used for humidity data
#define SENSOR_HEATINDEX_OFFSET 0 // used for heat index data

First, the comments are not correct :-)
Second, in the calculation method
float computeHeatIndex(float temperature, float percentHumidity) {
float hi;
temperature = temperature + SENSOR_TEMP_OFFSET; //include TEMP_OFFSET in HeatIndex computation too
temperature = 1.8*temperature+32; //convertion to *F
hi = 0.5 * (temperature + 61.0 + ((temperature - 68.0) * 1.2) + (percentHumidity * 0.094));

  • the temperature is getting his offset
  • the percentHumidity is not getting his offset: is this correct?

Something I don't find logic is that there is a SENSOR_HEATINDEX_OFFSET offset.
This value is computed, it is not a hardware reading with an error range of accuracy, so it must not have a offset in my opinion.

@cnerone
Copy link
Author

cnerone commented Apr 26, 2020 via email

@mfalkvidd
Copy link
Member

Dependencies

@ericvb yes dependencies are immensely troublesome.

Many of the MySensors examples do not need any external libraries. Those generally work quite well.

For example sketches that need other libraries, we've tried to make it easy for new users by verifying sketches against known versions on the libraries, and making that version of the libraries available in this repo. This method is not without troubles, but is so far the least troublesome way we've tried.

I have no experience with Jenkins, but I guess someone knowledgeable could spend the time required to "teach" Jenkins to compile with latest upstream version of the various libraries. I am not sure how much cost running additional builds would add, but it might be possible. I guess we'd also need some sort of build trigger (daily?) to check if the external libraries have been updated? (I think the MySensors builds only are run when new commits are made to the MySensors repository, but I am not sure.)

Doing a compile check will be insufficient though. What will happen if Jenkins detects a breakage? Who has the time, knowledge and hardware to troubleshoot and find a solution? Until such a solution is found and released, how do we inform users which library version they should use (and describe how to install that particular version)? What if something breaks, but that break is not detected by a build? How do we communicate build result information in a way that will reach inexperienced users?

If a breakage is detected and MySensors is updated, how do we inform people that they need to update their sketches and libraries (and how to do it)? Most users will be annoyed, confused and/or frustrated when their sketches, that previously worked fine, no longer compile, or when they compile and upload file but fail at runtime.

Dependencies is a mess. Sometimes I think we should just delete all examples that rely on external libraries. They've caused so much problems for so many people. But that would mean we'd be missing a lot of popular sensors (like the DHT) as well as a lot of good sensors.

I don't have any solutions unfortunately, but maybe we can work together to find something that is an improvement on what we have today.

@cnerone
Copy link
Author

cnerone commented Apr 26, 2020 via email

@ericvb
Copy link

ericvb commented Apr 27, 2020

@cnerone , that is indeed true. Maybe putting the reason as comment? It explains the 'why' for the readers/users :-)

@ericvb
Copy link

ericvb commented Apr 30, 2020

Hi @mfalkvidd
I did some research on the arduino platform.
Yes you are right, the use of arduino libraries is not ideal. The libraries are installed globally in the IDE and not specific for each scetch. Not like in Visual Studio where you can install nuget packages per project.
But there are 'some' workarounds.

  1. https://arduino.github.io/arduino-cli/sketch-specification/#libraryboards-manager-links
    In short, you can add, in comment, next to the library include, the library manager url link to search directly the library
// install the WiFiNINA library via Library Manager
// if using the Arduino IDE, click here: http://librarymanager#WiFiNINA
#include <WiFiNINA.h>

But there are some gotcha's

  • you can't use spaces in the url in case your library name contains them
  • you can't specify a specific library version
  1. https://arduino.github.io/arduino-cli/sketch-specification/#src-subfolder
    In short, inside your sketch folder with your ino file, you can have a subfolder named src with inside your depending libraries.
#include <SPI.h>
#include <MySensors.h>  
#include "src/dht/DHT.h"  // use included library DHT

<> will search the libs on the IDE default places
"" will search the libs in the src subfolder of the sketchfolder

This latest point makes it already easier for a beginner to compile the mysensors examples using specific external libraries.

The Arduino Web Editor has also the metadata file sketch.json where you can include the used libraries with their version tag.
https://arduino.github.io/arduino-cli/sketch-specification/#metadata
This should be ideal, but it's requiring a huge change in the IDE on premise...

I understand your concern for maintaining the examples.
As solution you could maybe provide default basic sketch examples with specific lib versions who will always work for the normal users of MySensors and add a section for advanced users in the documentation (and the example folder) with third-party examples more sophisticated with the side note that it is up to the author or the user, in case the author didn't do it, to make them compile with the latest mysensors lib and external libs if they aren't working anymore.
That's the same behaviour for third party plugins in other systems. If the author isn't maintaining his code, it's up to the user to do it in his place and to take over the job eventually...

All these points are constructively said to help to do even better. :-)
I wanted to share my distance sketch so that other users could directly see the problem with the original basic example. I did it by adding it to the forum, but it should be made easier to be able to do a pull request to change the documentation...
The forum is too big for new beginners, they will first start with the main examples and maybe loose time to get it working before turning to the forum for help, to discover that some one had the same problem and did solve it by an updated script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants