Skip to content
This repository has been archived by the owner on Feb 29, 2024. It is now read-only.

Swap Camera for Image Entity #512

Merged
merged 15 commits into from
Jul 13, 2023

Conversation

prairiesnpr
Copy link
Collaborator

This swaps the Camera entity to an Image entity.

@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.06 🎉

Comparison is base (2684f3b) 98.93% compared to head (cfe2bcd) 99.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #512      +/-   ##
==========================================
+ Coverage   98.93%   99.00%   +0.06%     
==========================================
  Files          16       16              
  Lines        1318     1302      -16     
==========================================
- Hits         1304     1289      -15     
+ Misses         14       13       -1     
Impacted Files Coverage Δ
...nts/husqvarna_automower/application_credentials.py 100.00% <ø> (ø)
...om_components/husqvarna_automower/system_health.py 100.00% <ø> (ø)
custom_components/husqvarna_automower/__init__.py 100.00% <100.00%> (ø)
custom_components/husqvarna_automower/calendar.py 100.00% <100.00%> (ø)
...stom_components/husqvarna_automower/config_flow.py 94.34% <100.00%> (+0.43%) ⬆️
custom_components/husqvarna_automower/const.py 100.00% <100.00%> (ø)
...m_components/husqvarna_automower/device_tracker.py 100.00% <100.00%> (ø)
custom_components/husqvarna_automower/entity.py 100.00% <100.00%> (ø)
custom_components/husqvarna_automower/image.py 100.00% <100.00%> (ø)
custom_components/husqvarna_automower/map_utils.py 100.00% <100.00%> (ø)
... and 2 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@prairiesnpr prairiesnpr marked this pull request as ready for review July 11, 2023 03:12
@Thomas55555
Copy link
Owner

  • It seems like there is text missing for "Enable Image"
Screenshot 2023-07-11 204529
  • Shouldn't we change the state of the image to image_last_updated. So that we have a timestamp there instead of, the mower's state?

@prairiesnpr
Copy link
Collaborator Author

  • It seems like there is text missing for "Enable Image"
Screenshot 2023-07-11 204529 * Shouldn't we change the state of the image to `image_last_updated`. So that we have a timestamp there instead of, the mower's state?

This should be good now, I went ahead and just migrated the key so we don't have camera anywhere anymore. Also reordered the class inheritance so state is back to being a timestamp.

Thanks, all good catches.

@Thomas55555
Copy link
Owner

I've removed the 'HusqvarnaAutomowerStateMixin` class. Think, thats not relevant anymore. Is there a need to change the tests?

@Thomas55555
Copy link
Owner

Look good. It feels like the image entity is faster than the camera entity.....
I've added some comments to the translations. I think there can be deleted some stuff. Can you have a look at this?
Further more I removed the HusqvarnaAutomowerStateMixin
I've you're okay with that, I think we can merge this. I assume, you have tested migration from the old to the new version? I didn't do that.

@prairiesnpr
Copy link
Collaborator Author

Look good. It feels like the image entity is faster than the camera entity..... I've added some comments to the translations. I think there can be deleted some stuff. Can you have a look at this? Further more I removed the HusqvarnaAutomowerStateMixin I've you're okay with that, I think we can merge this. I assume, you have tested migration from the old to the new version? I didn't do that.

I'll look this evening, I think the image entity is on average faster, but it seems to drag when using large images since we lose the ability to resize the image on the backend and that now has to be done in the front end. I left the resize code in, hopefully that will get added back in HA and we can implement it.

I'm not certain about the state translations, thinking those should be removed in #511? Will check to be sure.

I did test the migration, no issues found.

Didn't see any immediate concerns with 4dc8996 , but want to load it up in a codespace and just double check.

@Thomas55555
Copy link
Owner

That are the state translation for the image. So they are not related to the other PR.

@prairiesnpr
Copy link
Collaborator Author

OK, this should be ready to merge now.

@Thomas55555 Thomas55555 merged commit ff42674 into Thomas55555:main Jul 13, 2023
6 checks passed
@prairiesnpr prairiesnpr deleted the remove_camera_add_image branch July 13, 2023 17:51
@fredoche1810
Copy link

Hello @prairiesnpr @Thomas55555 ,
Thans for your update.
Before, I could take a snapshot, save it as JPG, and add the JPG to a notification.
Now that the camera entity doesn't exist anymore, I can't take a snapshot.
How can I attach a dynamic image to my notification now ?
Thanks in advance,

@prairiesnpr
Copy link
Collaborator Author

Hello @prairiesnpr @Thomas55555 , Thans for your update. Before, I could take a snapshot, save it as JPG, and add the JPG to a notification. Now that the camera entity doesn't exist anymore, I can't take a snapshot. How can I attach a dynamic image to my notification now ? Thanks in advance,

I'm using the same automation as before, just updated the camera entity to the image entity.

This is what I use now.

action:
  - variables:
      ha_url_var: !secret ha_url
      quest_update: '{{ states("sensor.aeinarr_error_quest_desc") }}'
  - service: notify.all_ios_devices
    data_template:
      message: '{{ state_attr("vacuum.aeinarr", "friendly_name") }} {{ quest_update }}'
      data:
        attachment:
          url: '{{ha_url_var}}{{ state_attr("image.aeinarr_map", "entity_picture") }}'
          content-type: jpg
          hide-thumbnail: false
  - service: notify.all_android_devices
    data_template:
      title: '{{ state_attr("vacuum.aeinarr", "friendly_name") }} Quest Update!'      message: '{{ state_attr("vacuum.aeinarr", "friendly_name") }} {{ quest_update }}'
      data:
        image: '{{ha_url_var}}{{ state_attr("image.aeinarr_map", "entity_picture") }}'

@fredoche1810
Copy link

Hello @prairiesnpr ,
Thanks for sharing your code.
How do you manage !secret ha_url ?
I tested your code with the internal url of homeassistant and it works if I'm on the LAN
I tested with the external url and it works when I'm outside the LAN.
How to make it working as well if connected to the LAN than when I'm outside the LAN ?
Thanks,

@prairiesnpr
Copy link
Collaborator Author

prairiesnpr commented Sep 5, 2023

In my setup, ha_url is just the external URL to my instance. I don't use the internal URL. That said, try dropping the URL, I don't think it's required.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants