ESPHome node for BH1750: problem at the default H-resolution mode2?

ESPHome
1

It looks like the current ESPHome implementation for the BH1750FVI has an issue when set at the H-resolution mode2 setting (which is the default setting in ESPHome): the currently given value is twice as high as what would be realistic.
The BH1750 supports three resolution settings: Low-res, High-res and High-res2.
This is set by the ESPHome configuration variable “resolution”:

resolution (Optional, string): The resolution of the sensor in lx. One of 4.0, 1.0, 0.5. Defaults to 0.5 (the maximum resolution).

So the H-res2 mode (maximum resolution) is used as default in the standard ESPHome BH1750 node.

I did some tests by putting the sensor in a completely closed closet with a light source (so a controlled environment), and this indeed confirms it: at several different illumination values the L-res and H-res values are always approximately identical, but the H-res2 values are always approximately twice as high.

So, even without doing a real calibration, I am pretty sure that the H-res2 values are currently unrealistic.
The BH1750FVI datasheet is not very clear on this aspect, but on page 11 of the Rev.D version it is shown that the H-res2 value should be divided by two:

BH1750FVI_H-res2

So I think that the standard ESPHome node for the BH1750 should by default divide the H-res2 value by two to get a realistic value.

Of course it is simple to correct for this, for instance by adding a multiply filter to the sensor, but shouldn’t the standard ESPHome node do this for us?
I understand that changing this in the standard ESPHome node could be a breaking change for many current implementations, so this might not be desirable. But I think it should at least be made clear at the BH1750 node web page that the H-res2 value should be divided by two if you need a more realistic value.

2

Did you try changing the measurement duration?

You might be running into a dynamic range issue.

By default the measurement_duration is set to 69 which will result into measurements up to 54612.5 lx for this sensor. For low-light situations consider to choose a higher measurement_duration up to 254 which will result in a maximum measurement range up to 14835 lx. For sunny scenes (for example outdoors with sunlight) use lower values down to 31 which will give you the maximum measurement range up to 121556 lx.

3

Thanks Tom.

Yes, sorry that I did not mention it, but I did change the MTres or “measurement_duration” value during my tests.
As I understand it, this MTres setting is meant to do a fine calibration of the sensor, for instance to cope with a covering in front of the sensor.
It appeared that changing this value does not have a large effect on the outcome, and the result is still that (without adding the correction for the H-res2 mode) the H-res2 mode value is always roughly twice as high as the H-res and L-res values for the current standard ESPHome BH1750 node.
Like I wrote, I did a test in a controlled environment (not in a real lab, but good enough for this purpose). I did measurements with both a low intensity and a high intensity light. This are the outcomes of the measurements:

ESPHome_BH1750FVI_test_20210823
ESPHome_BH1750FVI_test_20210823898×463 67.4 KB

As you can see the H-res2 values need to be divided by two to get them in line with the other resolution modes.

4

That’s pretty convincing. You should report the issue here:

5

Thanks Tom, I issued report on GitHub.

2 Likes
6

It has been more than seven weeks now since I issued the GitHub report, and apart from one response on GitHub adding the BH1750 label to the report nothing happened yet.
Since this is my first participation on GitHub I am not sure what to expect.
How long does it normally take to get a response from the code owners?

I am 100% sure that the current BH1750 values for the default HighRes-2 mode in the ESPHome BH1750 integration (resolution setting to 0.5) are incorrect (twice too high).
When looking at the Arduino BH1750 library by @claws (which is also referenced on the ESPHome BH1750 page) it can be seen that in there the value for this resolution mode is divided by two to get the realistic value:

BH1750_hres2_claws

Although I am far from an experienced coder I think I have an idea for how to modify the current ESPHome BH1750 integration code to correct it for this problem.
This part of theESPHome bh1750.cpp file:

void BH1750Sensor::read_data_() {
  uint16_t raw_value;
  if (!this->parent_->raw_receive_16(this->address_, &raw_value, 1)) {
    this->status_set_warning();
    return;
  }

  float lx = float(raw_value) / 1.2f;
  lx *= 69.0f / this->measurement_duration_;
  ESP_LOGD(TAG, "'%s': Got illuminance=%.1flx", this->get_name().c_str(), lx);
  this->publish_state(lx);
  this->status_clear_warning();
}

Should probably be modified into:

void BH1750Sensor::read_data_() {
  uint16_t raw_value;
  if (!this->parent_->raw_receive_16(this->address_, &raw_value, 1)) {
    this->status_set_warning();
    return;
  }

  float lx = float(raw_value) / 1.2f;
  lx *= 69.0f / this->measurement_duration_;
  if (this->resolution_ == BH1750_RESOLUTION_0P5_LX) {
      lx /= 2.0f;
  }
  ESP_LOGD(TAG, "'%s': Got illuminance=%.1flx", this->get_name().c_str(), lx);
  this->publish_state(lx);
  this->status_clear_warning();
}

Can I do anything else to support solving this issue?

1 Like
7

You should send a pull request with the changes of the code you did.
ESPHome had (and still has) a quite big backlog and pr’s get handled with some priority (compared to just issues).

If your pr is get’s quickly approved we could see the fix (with some luck) already in the upcoming version of esphome 2021.10 :rocket:

8

Thanks orange-assistant, but I am afraid that I do not feel comfortable enough to do this pull request myself. GitHub is all still new to me, and I don’t know what all is needed for this.

And also I am not completely sure about the best coding for this modification.
Initially I intended to use an “if” statement, but I was not sure how to combine this with the usage of the “this” pointers that are heavily used in the code.
So I changed it into a “switch” statement (following other examples in the code).

It does work like this for me locally, but it might still be that the code owners prefer using an “if” statement.
So I hope that one of the code owners or someone else can take this up.

9

You have everything (a github account). Can just modify the file within the browser and create a pull request for it. Very easy, just go ahead - you can’t break anything :wink:

Good thing is your pr will get one or more reviews before it’s get merged - nothing to worry here.

10

Thanks for the reassurance orange-assistant.
In the meantime I found out that the “if’ statement was indeed working as well: it was just a matter of me not understanding how to use the ESPHome “external_components” function. So I edited my code again in my previous post.
Currently I am doing some more local testing, and if it does work all fine I will give this pull request a try.

1 Like
11

OK, done my first pull-request :grinning:

3 Likes
12

Three more in October and you get a free t-shirt.

1 Like
13

Or can plant a tree instead :seedling:

1 Like
14

It is three weeks now since I did my pull-request, but I am still uncertain about the exact ESPHome GitHub process since I cannot find a clear explanation about this.
There are several trees for ESPHome on GitHub, and as I understand it now the main trees are: dev, beta and release.
It is now 10 days since the pull request has been merged into the ESPHome dev tree, but it is not yet merged into the release tree, nor the beta tree.
Can someone please explain how exactly this works and where I can find more information about the status?

1 Like
15

Which is totally fine. Beta and releases are just made monthly so a PR can “stuck” in dev for 3 and a 1/2 weeks easily.

In anyway your PR hit beta and will be in stable in two days.

1 Like
16

Thanks again orange-assistant :smiley:
With all my searches for more info about the ESPHome GitHub process I never landed on this ESPHome Beta website, but all is clear now.

1 Like
17

Also to mention the Dev (next) website: https://next.esphome.io

And you can also easily use beta or next esphome versions (mixed/parallel) together with the stable if they have features/fixes you like/need. :rocket:

And thanks for your PR @thusassistint - I use BH1750’s and will automagically get the improvements/fixes with the next stable in two days :muscle:

1 Like
18

Ah! Thanks again. Another useful link :+1:

You are welcome :smiley:
Just for everyone who is using the BH1750 in an automation: since this could be a breaking change be sure to check your automations after the update.
You might need to modify some triggers to get the same result again when you are using the default resolution mode HRes2 (0.5 resolution), by adding a multiplication by two.

2 Likes