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 SL20 #842

Merged
merged 3 commits into from
Jul 17, 2022
Merged

Add SL20 #842

merged 3 commits into from
Jul 17, 2022

Conversation

KTibow
Copy link
Contributor

@KTibow KTibow commented Jul 15, 2022

No description provided.

@bramstroker
Copy link
Owner

@KTibow Thanks for the contribution.
What is the reason why you couldn't use the measure tool for automatic recording of power values?
I see the color_temp.csv only contains 21 data points now which is not really up to the standards for the builtin power profile library.

@KTibow
Copy link
Contributor Author

KTibow commented Jul 16, 2022

The plug was very laggy in getting measurements, I was using the SonoffLAN integration, but I think it was using the cloud for some reason. If the tool could include a way to call update_entity, I would be able to use it.
PS: Is there any reason the tool is so complicated? The .env file is somewhat out of date, and there seems to be more options than needed, when you could just use Home Assistant to get data that will be used in Home Assistant.

@bramstroker
Copy link
Owner

@KTibow I think some other user got this issue before who also used a Sonoff. I'm pretty sure we discovered a a way to set the update frequency of the Sonoff. Let me check if I can find this conversation.

@bramstroker
Copy link
Owner

See #714, however this one i about the Sonoff flashed with ESPHome firmware. I am not sure if you can set a similar setting with the original firmware. I don't have a Sonoff myself so cannot check this.
I could add an optional service call to update_entity in the measure script but I prefer that you first have a look if the update frequency can somehow be decreased. Did you already test if update_entity gives a new power reading each time?

@bramstroker
Copy link
Owner

PS: Is there any reason the tool is so complicated? The .env file is somewhat out of date, and there seems to be more options than needed, when you could just use Home Assistant to get data that will be used in Home Assistant.

I am not sure what you mean with this. All the options in .env file has a purpose, there should not be any outdated settings that I know of. The measure tool is setup to be very easy to use for even non tech savy users, it contains a whole wizard and progress system which guide you through the process. Not sure how to improve on that significantly.

The whole point of the measure script is to walk through all possible brightness and color settings, and write data to the CSV files. That is not something which is build into HA in any way.

@KTibow
Copy link
Contributor Author

KTibow commented Jul 16, 2022

but I prefer that you first have a look if the update frequency can somehow be decreased

I've tried a couple of configuration changes, none of them seemed to work.

Did you already test if update_entity gives a new power reading each time?

Yes, I use this to check the power right now:

type: custom:mushroom-entity-card
entity: sensor.sonoff_10016407d0_power
tap_action:
  action: call-service
  service: homeassistant.update_entity
  data: {}
  target:
    entity_id: switch.sonoff_10016407d0

All the options in .env file has a purpose, there should not be any outdated settings that I know of.

Huh. I did have a bit of a bad time trying to use it, encountering stuff like

  • not being able to use backspace when I was entering into a text field
  • the descriptions being cut off instead of wrapping
  • having to change int(answers.get("num_lights", 1)) to int(answers.get("num_lights") or 1) because I wasn't measuring multiple lights
  • having to remove hardcoded things that were stopping me from having a larger gap in between measurements

@bramstroker
Copy link
Owner

Ok, I think it will be a good idea to make it possible to issue an update_entity call just before requesting the power meter value. Will add that somewhere in the next days. This feature will be optional and I will make a setting in .env where you can enable it.

I did not experience the bugs you noted yet. Looks like most of them seems bugs in the library I use for CLI wizard. https://github.com/magmax/python-inquirer. This is working fine on my Mac M1 macbook, and also on my ubuntu machine. There is not much I can do about that. Consistent cross platform CLI behaviour is a hard thing.

When you encounter small bugs or improvement with the measure tool which you can resolve PR's are welcome.

having to remove hardcoded things that were stopping me from having a larger gap in between measurements

There are certain limits coded to make sure the quality of the CSV files will be up to a certain standard.

@bramstroker
Copy link
Owner

Added the option with #850. You could update your local code to the updated master branch to give it a test run.

You need to set HASS_CALL_UPDATE_ENTITY_SERVICE=true in your .env file.

I have set an extra delay between update_entity and get_state now for 1 second, but we need to figure out a right value for this without delaying the measure session to much. You can change this line https://github.com/bramstroker/homeassistant-powercalc/blob/master/utils/measure/powermeter/hass.py#L25. It can also be a float, so to wait for 200ms, you can define 0.2.
Let me know what works for you.

@KTibow
Copy link
Contributor Author

KTibow commented Jul 16, 2022

I made a couple of modifications to the code to get it to work, trying out right now.

@bramstroker
Copy link
Owner

  • having to change int(answers.get("num_lights", 1)) to int(answers.get("num_lights") or 1) because I wasn't measuring multiple lights

This was actually broken. Also reported by other user #852. I was also able to reproduce. Is fixed in the codebase now.

@bramstroker
Copy link
Owner

CSV looks good now. Nice job. let's merge.

@bramstroker bramstroker merged commit 6e75936 into bramstroker:master Jul 17, 2022
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.

None yet

2 participants