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

Support for the AirThings Wave Plus #1656

Merged
merged 26 commits into from Aug 31, 2021

Conversation

jeromelaban
Copy link
Contributor

@jeromelaban jeromelaban commented Apr 3, 2021

What does this implement/fix?

Adds support for the AirThings Wave Plus BLE device.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Configuration change (this will require users to update their yaml configuration files to keep working)

Related issue or feature (if applicable): fixes esphome/feature-requests#147

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#1081

Test Environment

  • ESP32
  • ESP8266
  • Windows
  • Mac OS
  • Linux

Example entry for config.yaml:

BLE Scanner to find the device's MAC:

esp32_ble_tracker:
airthings_wave_plus:

Sensors:

  - platform: airthings_wave_plus
    mac_address: 'xx:xx:xx:xx:xx:xx'
    update_interval: 5min
    temperature:
      name: "Wave Plus Temperature"
    radon:
      name: "Wave Plus Radon"
    radon_long_term:
      name: "Wave Plus Radon Long Term"
    pressure:
      name: "Wave Plus Pressure"
    humidity:
      name: "Wave Plus Humidity"
    co2:
      name: "Wave Plus CO2"
    tvoc:
      name: "Wave Plus VOC"

Explain your changes

This PR is adding a AirThings device scanner, to find the BLE MAC address of the device, then a set of sensors provided by the device, polled every 5 minutes.

This PR adds a class, unit and icon related to radon measurement.

This is a new repository for me, so I'm pretty sure the esphome wizards will find things to say about it and how to make this PR better.

Tests and documentation are coming.

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@probot-esphome
Copy link

probot-esphome bot commented Apr 3, 2021

Hey there @esphome/core, mind taking a look at this pull request as its been labeled with an integration (sensor) you are listed as a codeowner for? Thanks!
(message by CodeOwnersMention)

@jeromelaban jeromelaban changed the title Develop/jela/waveplus Support for the AirThings Wave Plus Apr 3, 2021
@jeromelaban jeromelaban force-pushed the develop/jela/waveplus branch 4 times, most recently from 466ec49 to 53a838a Compare April 3, 2021 03:57
@jeromelaban
Copy link
Contributor Author

The code is looking better with some formatting applied, though the clang linter does not know how to find the "BLEDevice.h" header file, and requests changes that cannot be made (onConnect to on_connect for an overriden method).

Is there a way to hint the linter to find this arduino file ?

@jeromelaban jeromelaban force-pushed the develop/jela/waveplus branch 3 times, most recently from dc94b2d to 716e197 Compare April 3, 2021 20:55
@jeromelaban jeromelaban marked this pull request as ready for review April 3, 2021 21:15
@buxtronix
Copy link
Contributor

Could the BLE client part of this be managed by #1177 ?

@jeromelaban
Copy link
Contributor Author

jeromelaban commented Apr 4, 2021

@buxtronix it may end up being the base for the BLE Client, yes, but for now the integration in this PR is built around not keeping the BLE connection alive on purpose, so that the AirThings mobile app continues to work (I'll update the PR to mention that in the code and docs as well). I'm not sure if #1177 supports this mode yet.

@buxtronix
Copy link
Contributor

@buxtronix it may end up being the base for the BLE Client, yes, but for now the integration in this PR is built around not keeping the BLE connection alive on purpose, so that the AirThings mobile app continues to work (I'll update the PR to mention that in the code and docs as well). I'm not sure if #1177 supports this mode yet.

Yep, #1177 supports enabling and disabling individual client connections.

@jeromelaban
Copy link
Contributor Author

Got it, it's up to the sensor to enable/disable itself and wait for the connect event to get the attribute value. I don't follow all the pieces of that new API completely, but as long as there's a way to read an attribute raw value, it should be good.

This change delegates BLE managenement to BLEClientNode and BLEClient, instead of the integration initializating BLE itself.
@jesserockz jesserockz self-assigned this Aug 30, 2021
esphome/const.py Outdated
@@ -834,6 +838,7 @@
DEVICE_CLASS_TEMPERATURE = "temperature"
DEVICE_CLASS_POWER_FACTOR = "power_factor"
DEVICE_CLASS_PRESSURE = "pressure"
DEVICE_CLASS_RADON = "radon"
Copy link
Member

Choose a reason for hiding this comment

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

Device classes have to exist in Home Assistant before they can be added to ESPHome.
If it does not exists in HA, then DEVICE_CLASS_NONE should be used, or it should be left out of the named args of sensor_schema() (Like I modified)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I've removed class, that's good to know! That being said, I've been using it in home assistant with this for a while, and I have not noticed any warning or error message related to this. What are the consequences ?

Copy link
Member

Choose a reason for hiding this comment

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

There are no consequences, but we don't want to introduce classes that do not exist in HA. You are able to request the class be added in HA (not sure of the process, perhaps see a past PR that does this)

ESPHome Dev automation moved this from In Review to Reviewer Approved Aug 31, 2021
@jesserockz jesserockz merged commit 140ef79 into esphome:dev Aug 31, 2021
ESPHome Dev automation moved this from Reviewer Approved to Done Aug 31, 2021
@XBeg9
Copy link

XBeg9 commented Aug 31, 2021

will this be released soon?

@jesserockz jesserockz mentioned this pull request Sep 8, 2021
@Srki82
Copy link

Srki82 commented Sep 10, 2021

Hi,
Sorry for maybe a simple question but how do you enable the BLE Scanner to find the airthings MAC asdress? When I use the following code in my config.yaml file to find the airthings MAC:

esp32_ble_tracker:
airthings_wave_plus:

I get an error for the row airthings_wave_plus:
I have installed the latest beta 2021.9.0b2 but it is like airthings_wave_plus cannot be included into the code.
What am I missing?

@jesserockz
Copy link
Member

@Srki82 Please look at the example carefully https://next.esphome.io/components/sensor/airthings_ble.html#device-discovery

The discovery YAML key is not airthings_wave_plus

@Srki82
Copy link

Srki82 commented Sep 12, 2021

@jesserockz thanks! I followed the instructions and I managed to find the MAC adress and then I got the sensors inside Home Assistant. The only issue now is that all sensors shows the state "unknown".
I assume its not communicating with the airthing plus unit for some reason. Any further tips?

@jeromelaban
Copy link
Contributor Author

jeromelaban commented Sep 12, 2021

@jesserockz thanks! I followed the instructions and I managed to find the MAC adress and then I got the sensors inside Home Assistant. The only issue now is that all sensors shows the state "unknown".
I assume its not communicating with the airthing plus unit for some reason. Any further tips?

If all the states are unknown, this means there's a communication failure somewhere. I'd say this PR is not the right place to discuss this, and a new issue would be more interesting to provide additional information. Can you open a new one with debugging information from the ESP?

@esphome esphome locked and limited conversation to collaborators Sep 13, 2021
ESPHome Dev automation moved this from Done to In Review Sep 23, 2023
ESPHome Dev automation moved this from In Review to Reviewer Approved Sep 23, 2023
ESPHome Dev automation moved this from Reviewer Approved to Done Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging this pull request may close these issues.

AirThings BLE Radon Sensor
8 participants