bramstroker / homeassistant-powercalc

Custom component to calculate estimated power consumption of lights and other appliances
MIT License
937 stars 250 forks source link

Easier measurement process for new device #1236

Closed knudsvik closed 1 year ago

knudsvik commented 1 year ago

Ideally I would want an easier way to add new equipment. The perfect speaker flow in my mind would look like this (after running the docker command):

Then the script could play a test sound from a web address, turn the volume up in cycles, mute it all the while measuring power. In the end it could create the model.json file. This would be a standardized way securing consistency as well! Potentially one could also try out the different attributes available on the device automatically, like loudness, bass levels etc and also incorporate that in the model.

knudsvik commented 1 year ago

This page has some demo sounds to calibrate sound systems. The pink noise could be a good choice: "It is an excellent signal for comparing the effect of any change you introduce in your sound system in the A/B test fashion."

bramstroker commented 1 year ago

Yes, there can definitely be a lot improved upon the speaker measurement.

I have chosen to keep it simple now, because I didn't know if a lot of smart speakers were being provided to the library yet, and it would take a significant amount of work and refactoring to facilitate two different measurement flows (light and smart speaker). I see you want to have a go into developing the measure script. Here are some pointers. I would suggest to take it small steps at a time.

We need to somehow extract the light measurement logic into a separate class. So we can also introduce smart speaker logic. Then some questions / answers could be asked in the main measurement class (i.e. select power measuring entity, generate model.json yes/no etc.). And depending on equipment selection we can dispatch the actual measure logic to the respective class.

I am not sure if the HA API exposes device information (pretty sure it doesn't). So I suggest to just keep that the same as the light selection, where just all the entities of domain light are listed. Smart speakers can just list all entities of domain media_player. It won't be hundreds of them, so selection of manufacturer/model would be overkill.

Not sure how to play a test sound using HA service which works for alle media players and HA installations.

For the submittion of HA address etc. I would leave it to the env approach right now, which works fine for all users now. We can maybe improve upon that in a future iteration, but for now keep it lean and mean.

bramstroker commented 1 year ago

@knudsvik I have worked for a few hours to completely refactor the measure tool to cleanup the measure main script and move logic to respective places for less cohesion and to adhere to single response principle. See #1286

Few things I have done:

Are you able to give this a test run by checkout out this branch (fix/measure-code-standards)? See if this works correctly for you (particularly the streaming part). Let me know what you think.

knudsvik commented 1 year ago

@bramstroker I get an error (using pyenv virtual environment and python 3.10.4)

Traceback (most recent call last):
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/measure.py", line 11, in <module>
    import config
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/config.py", line 42, in <module>
    SELECTED_MEDIA_CONTROLLER = config("MEDIA_CONTROLLER", cast=Choices([t.value for t in MediaControllerType]), default=MediaControllerType.HASS)
  File "/Users/thorjanknudsvik/.pyenv/versions/testMeasure/lib/python3.10/site-packages/decouple.py", line 199, in __call__
    return self.config(*args, **kwargs)
  File "/Users/thorjanknudsvik/.pyenv/versions/testMeasure/lib/python3.10/site-packages/decouple.py", line 83, in __call__
    return self.get(*args, **kwargs)
  File "/Users/thorjanknudsvik/.pyenv/versions/testMeasure/lib/python3.10/site-packages/decouple.py", line 77, in get
    return cast(value)
  File "/Users/thorjanknudsvik/.pyenv/versions/testMeasure/lib/python3.10/site-packages/decouple.py", line 261, in __call__
    raise ValueError((
ValueError: Value not in list: <MediaControllerType.HASS: 'hass'>; valid values are ['dummy', 'hass']
bramstroker commented 1 year ago

You have to add MEDIA_CONTROLLER to env. See env.dist

bramstroker commented 1 year ago

@knudsvik Can you find some time to have a look? I would really like a second pair of eyes before I merge this PR.

knudsvik commented 1 year ago

@bramstroker Some findings:

Traceback (most recent call last):
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/measure.py", line 277, in <module>
    main()
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/measure.py", line 269, in main
    measure.start()
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/measure.py", line 115, in start
    standby_power = self.runner.measure_standby_power()
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/runner/speaker.py", line 77, in measure_standby_power
    self.media_controller.turn_off()
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/media_controller/hass.py", line 33, in turn_off
    self.client.trigger_service('media_player', 'turn_off')
  File "/Users/thorjanknudsvik/.pyenv/versions/powercalc/lib/python3.10/site-packages/homeassistant_api/client.py", line 185, in trigger_service
    data = self.request(
  File "/Users/thorjanknudsvik/.pyenv/versions/powercalc/lib/python3.10/site-packages/homeassistant_api/rawapi.py", line 59, in request
    return self.response_logic(resp)
  File "/Users/thorjanknudsvik/.pyenv/versions/powercalc/lib/python3.10/site-packages/homeassistant_api/rawapi.py", line 64, in response_logic
    return processing.process()
  File "/Users/thorjanknudsvik/.pyenv/versions/powercalc/lib/python3.10/site-packages/homeassistant_api/processing.py", line 64, in process
    raise RequestError(self.request.content)
AttributeError: 'Processing' object has no attribute 'request'
bramstroker commented 1 year ago

@bramstroker Some findings:

  • I would prefer to keep Other as an alternative to measuring other types of equipment (I have used it for lot's of stuff and plan to add even more).

You can just start using the average {time} option, I have leaved that option and that would do exactly the same.

  • When asking for full model name you could ask specifically for Manufacturer and Model? Also this question should not be necessary when not creating the model.json file?

This is the full name of the light model, as on the package or advertised. It will be used in the name field in model.json, and this is used to display in the supported models listing. There is no way we could know this and it's needed for the model.json generation.

  • The script asks me for power meter even though I have specified it in the .env file. I guess it's for the model.json file? Could it be read from there instead?

What exactly did you specify in .env? This is the same as above. We need to know which manufactures/model of powermeter the user is using for the measurement session. This is the measure_device field in model.json.

  • When typing on the questions I cannot use backspace to delete something I have written wrong..

I have this same issue. This bug was already existing as I didn't change anything related to the CLI wizard. I'll create a separate bug report for this. We are relying on https://github.com/magmax/python-inquirer for that, so I will see if we can update that package. When this doesn't solve I can create a bug report there. Which OS/terminal do you use?

  • Even though the script starts measuring I could not hear any sound from my speaker (Sonos bookshelf). Maybe there could be a test run like setting the volume to 20% and playing noise for 10 seconds and have the user acknowledge that he/she hears the noise before starting the actual measurements? The script is able to change the volume on the speaker. From the media_player entity in HA I see that it tries to play this file: http://IP:8123/api/tts_proxy/91bde800add81daea2ac95ce8968b193fd876e49_nb-no_a9c18110b0_cloud.mp3

This was working for me on my Harman Kardon Citation. Could you try in developer tools -> services?

service: media_player.play_media
data:
  entity_id: media_player.hk_citation_multibeam
  media_content_id: https://assets.ctfassets.net/4zjnzn055a4v/5d3omOpQeliAWkVm1QmZBj/308d35fbb226a4643aeb7b132949dbd3/g_pink.mp3
  media_content_type: audio
  • At the end of the cycle, the volume should be set to something else than 100%, else the user would jump through the roof by the next time the speaker is to be used.

Good idea ;-), I have changed the code to set the volume to 10 after measurement session completes.

  • When not specifying that I want model.json the last printed statement is that I can find exported files in the export directory, but nothing is there.

Fixed now

  • After summing up the measurements I get this error message (only when specifying that I want model.json):
Traceback (most recent call last):
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/measure.py", line 277, in <module>
    main()
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/measure.py", line 269, in main
    measure.start()
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/measure.py", line 115, in start
    standby_power = self.runner.measure_standby_power()
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/runner/speaker.py", line 77, in measure_standby_power
    self.media_controller.turn_off()
  File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/media_controller/hass.py", line 33, in turn_off
    self.client.trigger_service('media_player', 'turn_off')
  File "/Users/thorjanknudsvik/.pyenv/versions/powercalc/lib/python3.10/site-packages/homeassistant_api/client.py", line 185, in trigger_service
    data = self.request(
  File "/Users/thorjanknudsvik/.pyenv/versions/powercalc/lib/python3.10/site-packages/homeassistant_api/rawapi.py", line 59, in request
    return self.response_logic(resp)
  File "/Users/thorjanknudsvik/.pyenv/versions/powercalc/lib/python3.10/site-packages/homeassistant_api/rawapi.py", line 64, in response_logic
    return processing.process()
  File "/Users/thorjanknudsvik/.pyenv/versions/powercalc/lib/python3.10/site-packages/homeassistant_api/processing.py", line 64, in process
    raise RequestError(self.request.content)
AttributeError: 'Processing' object has no attribute 'request'

Not sure about this one, seems there was a problem with the HA API at that time. Did you have this error each time or only once?

bramstroker commented 1 year ago

CLI backspace issue has been resolved by upgrading pyinquirer and readchar libraries.

I was able to reproduce your last error AttributeError: 'Processing' object has no attribute 'request'. Looking into that now.

bramstroker commented 1 year ago

Processing error has also been fixed.

knudsvik commented 1 year ago

@bramstroker unfortunately that introduced a new error on line 75 in config.py: SELECTED_DEVICE_TYPE not found. Declare it as envvar or define a default value.

bramstroker commented 1 year ago

I see, I have added possibility to select device type in .env, because I have issue with Python debugger in combination with the CLI wizard. So that allows me to predefine all answers of the wizard. Anyway it should be fixed now.

knudsvik commented 1 year ago
  1. When I say no to not create model.json file I should not need to be asked what model media player I have, since the information is not used for anything. not a critical issue, but a small thing that could make the experience easier ;)
  2. Still no sound... This did not work on my Sonos One:
    service: media_player.play_media
    data:
    entity_id: media_player.hk_citation_multibeam
    media_content_id: https://assets.ctfassets.net/4zjnzn055a4v/5d3omOpQeliAWkVm1QmZBj/308d35fbb226a4643aeb7b132949dbd3/g_pink.mp3
    media_content_type: audio

But this worked fine:

service: media_player.play_media
data:
  media_content_type: music
  media_content_id: >-
    https://assets.ctfassets.net/4zjnzn055a4v/5d3omOpQeliAWkVm1QmZBj/308d35fbb226a4643aeb7b132949dbd3/g_pink.mp3
target:
  entity_id: media_player.bad
  1. I get a new error message now at the end (after the summary table) when saying yes to create model.json:
    If this happened, please report it at https://github.com/GrandMoff100/HomeAssistantAPI/issues with the request status code and the request content
    Traceback (most recent call last):
    File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/measure.py", line 284, in <module>
    main()
    File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/measure.py", line 276, in main
    measure.start()
    File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/measure.py", line 121, in start
    standby_power = self.runner.measure_standby_power()
    File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/runner/speaker.py", line 79, in measure_standby_power
    self.media_controller.turn_off()
    File "/Users/thorjanknudsvik/Desktop/temp/powercalctest/homeassistant-powercalc/utils/measure/media_controller/hass.py", line 33, in turn_off
    self.client.trigger_service("media_player", "turn_off", entity_id=self._entity_id)
    File "/Users/thorjanknudsvik/.pyenv/versions/3.10.4/lib/python3.10/site-packages/homeassistant_api/client.py", line 185, in trigger_service
    data = self.request(
    File "/Users/thorjanknudsvik/.pyenv/versions/3.10.4/lib/python3.10/site-packages/homeassistant_api/rawapi.py", line 59, in request
    return self.response_logic(resp)
    File "/Users/thorjanknudsvik/.pyenv/versions/3.10.4/lib/python3.10/site-packages/homeassistant_api/rawapi.py", line 64, in response_logic
    return processing.process()
    File "/Users/thorjanknudsvik/.pyenv/versions/3.10.4/lib/python3.10/site-packages/homeassistant_api/processing.py", line 73, in process
    raise UnexpectedStatusCodeError(self.response.status_code, self.response.content)
    homeassistant_api.errors.UnexpectedStatusCodeError: Homeassistant return response with an unrecognized status code 500.
    b'500 Internal Server Error\n\nServer got itself in trouble'
bramstroker commented 1 year ago

I have tried changing content_type from audio to music.

The second error is when the turn off service is called, before it tries to measure standby power. Not sure why this is failing for you. 500 internal server error indicates HA raises an error, maybe you can find something in HA error logs?

You can also try that one with YAML in developer tools services. Is this working for you? This is working for me btw. Tested with 5 different media player entities.

service: media_player.turn_off
data:
  entity_id: media_player.chromecast_woonkamer
bramstroker commented 1 year ago

@knudsvik Did you see my comments above? Are you able to verify these. When everything is ironed out I can merge the PR and issue a new release for the improved measure tool. And happy holidays.

knudsvik commented 1 year ago

Hi @bramstroker , sorry for not getting back to you earlier. I tried the new script now: it's playing the pink noise now :)

a couple of quick comments here as well (maybe future work):

  1. The power usage is quite stable after a few seconds. If the script sees that it could maybe just go to the next measurement? Reason for shortening the cycles: The noise is really bad for the ears (and the family) when the volume cranks up haha
  2. Still think my comment above is valid: when I choose not to generate the json file, the script should not ask me for model of the speaker as it is not used for anything anyhow.

But good work!

bramstroker commented 1 year ago

Thanks for testing. Great the pink noise is also playing for you now.

  1. The power usage is quite stable after a few seconds. If the script sees that it could maybe just go to the next measurement? Reason for shortening the cycles: The noise is really bad for the ears (and the family) when the volume cranks up haha

Maybe I will add that in the future. The measure session is relatively short and an active noise cancelling headphone does wonders ;-).

  1. Still think my comment above is valid: when I choose not to generate the json file, the script should not ask me for model of the speaker as it is not used for anything anyhow.

This question is asked to determine what the model id is. This is used as directory name in the export directory. I have no way to retrieve that information from HA api unfortunately. When you have enabled model.json generation an additional question will be asked "Specify the full Smart speaker model name"

But good work!

Thanks

bramstroker commented 1 year ago

Has been merged into master.

Thanks for helping test and providing feedback on this one.