EverythingSmartHome / everything-presence-lite

Everything Presence Lite
82 stars 32 forks source link

missing "i2c_id" for bh1750 and missing "id" for i2c #28

Open kloppy opened 8 months ago

kloppy commented 8 months ago

Hi, total noob here with regards to ESPHome. Just got my 4 pcs EPL, installed ESPHome and started playing around. I wanted to add another i2c sensor and thus added a second i2c bus to the config. This resulted in the illuminance sensor stopping to work.

As far as I can see, neither the first i2c bus nor the sensor have the corresponding id field set, which would remedy this issue.

Thank you for this awesome little product, I love it!

EverythingSmartHome commented 8 months ago

Glad you are enjoying it!

There is currently no need to have an ID set for the i2c bus on the stock firmware as there is only one. If you are adding more sensors then that means your editing the config anyways at which point you can just add it to your custom config

kloppy commented 8 months ago

Yes I can do that, but then I will miss potential updates you make to the yaml.

It is not needed, but it would not take anything away from people using the stock config, while making life easier for people who want to add a second i2c bus.

Consider it :)

EverythingSmartHome commented 8 months ago

Right but your going to need to create your own config regardless to add additional sensors and the additional i2c bus no?

kloppy commented 8 months ago

workaround below

Of course, maybe we misunderstand each other here. My impression is that the EPL is intended to be extensible, hence the exposed GPIOs and the open source firmware - it is why people like me like it and buy it.

The provided package is a great starting point to either copy and modify it or to extend it, the latter of which is just a much much more elegant solution. The custom config stays small and benefits from potential updates upon recompilation. Also, just as a matter of principle, if you can uniquely identify an entity with an id, it's good practice to do so.

If you disagree, or you don't see the benefit, feel free to close the issue. I just wanted to point out a small and easy improvement.

In any case, here is the solution to the issue. The following code (placed below the package import) overrides the package i2c definition to add an id, also it extends the bh1750 definition to reference this bus id. At last, it adds a second i2c bus and a bme280. This is what I wanted and it works.

# override the i2c definition in everything-presence-lite-ha.yaml
# to include an id for the bus, also add a second i2c bus which 
# does not break the bh1750
i2c:
  - id: bus_epl
    sda: 21
    scl: 22
    scan: true
  - id: bus_custom
    sda: 26
    scl: 27
    scan: true

# extend the definition for the bh1750 illuminance sensor 
# to reference the i2c bus by id, also add a bme280 
# environment sensor
sensor:
  - id: !extend illuminance_sensor
    i2c_id: bus_epl
  - id: environment_sensor
    i2c_id: bus_custom
    platform: bme280
    address: 0x76
    update_interval: 30s
    temperature:
      name: "Temperature"
    pressure:
      name: "Pressure"
    humidity:
      name: "Humidity"