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

Add support for AZ-Instrument 7798 CO2 meter/datalogger #4672

Merged
merged 10 commits into from
Dec 18, 2018

Conversation

adebeun
Copy link
Contributor

@adebeun adebeun commented Dec 18, 2018

Implemented sensor interface for the AZ-Instrument 7798 CO2/temperature/humidity meter/datalogger.
See: https://www.az-instrument.com.tw/az-instrument/en/productsinfo/189.html
Tested with a Wemos D1 mini. (Can be fitted neatly inside the instrument.)

Copy link
Collaborator

@ascillato2 ascillato2 left a comment

Choose a reason for hiding this comment

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

Hi, thanks for sharing this. There are several points that need be addressed. This PR will not compile due problems in language files.

Platformio.ini should not be changed. Those changes has nothing to do with your driver.
My_User_config.h should be left as default. Too much changes not related to this driver. Besides the key that enable this driver should be left commented out.
There is missing some translations and also there is a conflict in one of the files that needs to be resolved. Without all those changes, can't be merged due this can't compile.

Thanks

adding line for new sensor but leaving it commented out.
Copy link
Collaborator

@andrethomas2 andrethomas2 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 the PR, but I agree with @ascillato2

I'm inclined to think you should create a new PR which only has the necessary changes as opposed to trying to clean this one up.

@adebeun
Copy link
Contributor Author

adebeun commented Dec 18, 2018

I made the changes as requested, reverting my_user_config and platformio.ini to their original state, but adding the new sensor commented out. I retrieved the updated bg-BG.h and added my 2 new lines. However it is still showing as conflicted, I don't know how to fix that.

@andrethomas
Copy link
Contributor

The conflict needs to be resolved

image

So just click on resolve conflict and edit it accordingly.

Copy link
Contributor

@andrethomas andrethomas left a comment

Choose a reason for hiding this comment

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

Also, please use the original RELEASENOTES.md as this is not included in the 6.4 release and your PR is proposed for the current development codebase

Copy link
Collaborator

@andrethomas2 andrethomas2 left a comment

Choose a reason for hiding this comment

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

Also, please use the original RELEASENOTES.md as this is not included in the 6.4 release and your PR is proposed for the current development codebase

Fix temperature conversion to still use ConvertTemp() even if meter sends the value in F
@arendst arendst merged commit 3229148 into arendst:development Dec 18, 2018
gemu2015 pushed a commit to gemu2015/Sonoff-Tasmota that referenced this pull request Jan 27, 2019
Add support for AZ-Instrument 7798 CO2 meter/datalogger
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.

6 participants