bogde / HX711

An Arduino library to interface the Avia Semiconductor HX711 24-Bit Analog-to-Digital Converter (ADC) for Weight Scales.
MIT License
896 stars 538 forks source link

Including head in HX711.cpp #149

Open MarioJose opened 5 years ago

MarioJose commented 5 years ago

Line 11 in HX711.cpp

#include <HX711.h>

is including HX711 library in system installation (<> mark), not in library ("" mark) folder. If using library in local folder or old version of library in system installation, this include line get error when compiling.

I saw in other libraries and I think it is better include with (quote mark):

#include "HX711.h"

Is it make sense? I have trouble using HX711 in local folder and I needed change this line in my local installation.

I think part of @csoeder problem #140 maybe because of this include line. @leon0399 make a pull request #104 about it too.

amotl commented 5 years ago

Dear Mario José,

thanks for writing in. If this will help you and others for specific things we haven't been able to sort out, this totally makes sense. If you have investigated and confirmed the problem well enough to be confident this will improve the library and not break it for others, we will be happy to see #104 resembled as an updated pull request against the current master branch.

If @bogde is around these days and will not vote otherwise, we believe there should be a way to get this into master without efforts. Shipping an updated release to the Arduino Library Manager Repository would make sense then.

With kind regards, Andreas.

MarioJose commented 5 years ago

Hi, @amotl,

I understand your point about stability of the library.

I tested with Arduino IDE and Atom+PlatformIO.

In Arduino IDE, I needed put HX711 library in sketch folder to ensure that my code is using correct version of the library (I removed too HX711 from system library (arduino/library) ). For my understand, "" marks make compiler search first library in local folder before search library folder. When I compile the code, I get the error that compiler not found HX711 library (call in HX711.cpp). Then, updating HX711.cpp call for header make compiler works.

In Atom+PlatformIO, the compiler works regardless HX711.cpp call is with <> or "". But, PlatformIO deal with libraries in /lib as a system library. I believe because that the compiler works.

For my basic knowledge (I'm c programmer beginner), pre processor will compile library first of main code. Header of HX711 is the same folder of HX711.cpp, them I think that update <> to "" will not make library broken. Maybe I'm wrong. I search for how some other library call header and all the call is with "" (like Adafruit libraries, for instance).

Best regards!

Mario

amotl commented 5 years ago

Dear Mario,

what you are telling us sounds totally reasonable. Sorry that I am currently not able to got into the details. I am humbly asking @bogde to review this and I am also humbly asking you to put this oneliner-change into a pull request against current master.

Thanks, Andreas.

r-a-i commented 5 years ago

I ran into this same issue.

The #include directive caused a compilation failure because I did not have (nor did I wish to have) this library installed as part of my IDE. I resolved it as suggested by @MarioJose by replacing the directive to #include "HX711.h".

For me, the problem arose because I cloned the git repo as a submodule into another git repo. I do this instead of installing the libraries in the IDE so that I can easily port my code between machines (and users) without having to have each instance configure the IDE and install the dependencies.

Please merge this pull request into master.

amotl commented 5 years ago

Dear @r-a-i,

I ran into this same issue. The #include <HX711.h> directive causes a compilation failure e.g. when cloning the git repo as a submodule into another git repo.

We hear you.

Please merge this pull request into master.

I believe there is no pull request about this yet. May I humbly ask you to create one in order to make it easier for @bogde to apply these changes?

Thanks in advance and with kind regards, Andreas.

amotl commented 5 years ago

I believe there is no pull request about this yet.

Ah, there it is already as referenced by @MarioJose. Sorry for overlooking this.

@leon0399 made a pull request #104 about it too.

However, this has happened before the large refactoring which took place in spring 2019. So, this pull requests shows conflicts in HX711.cpp.

In order to resolve this issue, I am humbly asking for an updated pull request. I believe your contribution totally makes sense, I just can't find enough time right now.

r-a-i commented 5 years ago

Dear @amotl, thanks for reviewing this.

I've created the pull request. Hope this helps.

amotl commented 4 years ago

Hi there,

if we are all happy with the outcome of this, we might want to finally close the issue.

Thanks already and with kind regards, Andreas.

r-a-i commented 4 years ago

Closing this sounds good to me