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

Refactor people counter to a number component #83

Merged
merged 4 commits into from
Jan 3, 2022
Merged

Conversation

CarsonF
Copy link
Collaborator

@CarsonF CarsonF commented Jan 2, 2022

This replaces the sensor for people counts with a number. Numbers are controllable which makes it much easier to change the state in HA UI & automation without having to create lambda API services.

This number is configured for people counting so min=0, step=1, max is configurable.

It is an instance of a new PersistedNumber object I created, that saves/restores the value using a native esphome API. This removes our explicit EEPROM dependency.

This flash write is debounced and can be user configured, defaults to once a minute.
https://esphome.io/components/esphome.html#adjusting-flash-writes
In testing it seems pending writes are still saved on graceful reboot.
Note that ESP8266 have this disabled by default, so...

esp8266:
  restore_from_flash: true

Since this count was the only value that was persisted/"restored", I removed the restore_values config option in favor of restore_value on the number component.

I couldn't figure out how to reference the roode platform outside of its platform setup, so for now it's just there. I suspect I just missed something and it can be moved to an independent number entry with the roode platform.

number:
  - platform: roode
    people_counter:
      name: $friendly_name people counter
      max_value: 5 # default 10

Screen Shot 2022-01-01 at 6 14 54 PM

Copy link
Owner

@Lyr3x Lyr3x left a comment

Choose a reason for hiding this comment

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

Good job! Thanks a lot for that awesome refactor!
I added some minor request changes though, nothing big.
To use a proper number entity in the esphome config, you should carve out the number component analog to text_sensor etc. That way you can define the number like this:

number:
  - platform: roode
    people_counter:
      name: $friendly_name people counter
      restore_value: true

This should work:
number.py:

import esphome.codegen as cg
import esphome.config_validation as cv
from esphome.components import number
from typing import OrderedDict
from esphome.const import (
    CONF_ID,
    CONF_ICON,
    CONF_ACCURACY_DECIMALS,
    CONF_MAX_VALUE,
    CONF_RESTORE_VALUE,
)
from . import Roode, CONF_ROODE_ID

roode_ns = cg.esphome_ns.namespace("roode")
PersistedNumber = roode_ns.class_("PersistedNumber", number.Number, cg.Component)

DEPENDENCIES = ["roode"]
CONF_PEOPLE_COUNTER = "people_counter"
TYPES = [CONF_PEOPLE_COUNTER]

CONFIG_SCHEMA = cv.Schema(
    {
        cv.GenerateID(CONF_ROODE_ID): cv.use_id(Roode),
        cv.Optional(CONF_PEOPLE_COUNTER): number.NUMBER_SCHEMA.extend(
            {
                cv.GenerateID(): cv.declare_id(PersistedNumber),
                cv.Optional(CONF_ICON, default="mdi:counter"): cv.icon,
                cv.Optional(CONF_MAX_VALUE, 10): cv.int_range(1, 255),
                cv.Optional(CONF_RESTORE_VALUE): cv.boolean,
            }
        ),
    }
)


async def people_counter(config: OrderedDict):
    """People counter, which is a persisted number, but min/max/step are specific to people counts"""
    var = cg.new_Pvariable(config[CONF_ID])
    await cg.register_component(var, config)
    await number.register_number(
        var, config, min_value=0, max_value=config[CONF_MAX_VALUE], step=1
    )
    if CONF_RESTORE_VALUE in config:
        cg.add(var.set_restore_value(config[CONF_RESTORE_VALUE]))
    return var

async def to_code(config):
    hub = await cg.get_variable(config[CONF_ROODE_ID])
    if CONF_PEOPLE_COUNTER in config:
        counter = await people_counter(config[CONF_PEOPLE_COUNTER])
        cg.add(hub.set_people_counter(counter))

Then you can remove the whole people counter from __init__.py

components/roode/__init__.py Outdated Show resolved Hide resolved
components/roode/__init__.py Outdated Show resolved Hide resolved
components/roode/__init__.py Outdated Show resolved Hide resolved
components/roode/persisted_number.h Outdated Show resolved Hide resolved
namespace esphome {
namespace roode {
auto PersistedNumber::control(float newValue) -> void {
this->publish_state(newValue);
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
this->publish_state(newValue);
ESP_LOGI("People Counter", "Updating people counter to: %d", (int)newValue);
this->publish_state(newValue);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Parent has one so I omitted because I didn't want to duplicate
https://github.com/esphome/esphome/blob/b5a0e8b2c0d016ea384f4f962e0f0e4c61a8dc85/esphome/components/number/number.cpp#L36

It's debug level, but I think logs are only relevant at the debug level anyways. Otherwise you just can observe the current state.

I'll add it though if you really want an info level as well.

Copy link
Owner

@Lyr3x Lyr3x Jan 2, 2022

Choose a reason for hiding this comment

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

Yes because the parent has its on DEBUG i have this added previously as INFO because i need this log very often when testing things and only check the terminal. I dont use HA when developing stuff. That's a essential log line.

When setting the level to DEBUG there is a lot going one which i only do if i really want to see step by step whats going on.

Copy link
Collaborator Author

@CarsonF CarsonF Jan 2, 2022

Choose a reason for hiding this comment

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

K LMK what you think of new code. Since it has nothing to do with roode or people counting now I left it as a float.

components/roode/__init__.py Outdated Show resolved Hide resolved
components/roode/__init__.py Outdated Show resolved Hide resolved
@CarsonF
Copy link
Collaborator Author

CarsonF commented Jan 2, 2022

Then you can remove the whole people counter from __init__.py

Yeah I tried all of that yesterday with a number file—I couldn't access roode config. I think I was missing this line to make it work:

 cv.GenerateID(CONF_ROODE_ID): cv.use_id(Roode),

I also wanted to move persisted number python code to a separate persisted_number.py but I think esphome tried to load it because of MULTI_CONF so I gave up and merged it back into the file.

@Lyr3x
Copy link
Owner

Lyr3x commented Jan 2, 2022

In ESPHome sense it is a number so just put it to the number.py like above and we are good to go 👍

@CarsonF CarsonF force-pushed the number branch 2 times, most recently from d8dbe36 to 8b40119 Compare January 2, 2022 16:17
@CarsonF CarsonF requested a review from Lyr3x January 2, 2022 16:20
…ber without previous state

Follows suit of TemplateNumber barring initial value property
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.

2 participants