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

[Bug]: rstrip / replace bugs in BETA. #63

Closed
suredesigntshirts opened this issue Jul 31, 2023 · 3 comments · Fixed by #70
Closed

[Bug]: rstrip / replace bugs in BETA. #63

suredesigntshirts opened this issue Jul 31, 2023 · 3 comments · Fixed by #70
Assignees
Labels
bug Something isn't working

Comments

@suredesigntshirts
Copy link

suredesigntshirts commented Jul 31, 2023

Description

rstrip bug

The argument you pass to rstrip in python is a set of characters to strip, not a string to strip.

Hence: rstrip('_1').rstrip('_2').rstrip('_3') will replace any of these characters at the end of a string [123_]

So a camera which is named hello123_ would get changed to hello

technically rstrip('_123') would do exact same as code you have in your BETA blueprint.

Anyways it's wrong.


replace "camera." bug

Additionally the replace('camera.') would also error on a camera named camera.outdoor.camera.321 would be replaced to outdoor.

To resolve, I've changed this code from

  camera: "{{ input_camera.rstrip('_1').rstrip('_2').rstrip('_3') | replace('camera.', '') }}"

to

  camera: "{{ input_camera | regex_replace('^camera\.|_\d+$') }}"

Regex Explained

Docs: https://www.home-assistant.io/docs/configuration/templating/#regular-expressions

^camera\. = ^ is saying only look at start of string. Then look for camera.. You need to escape the period, hence the slash.

| = checks for another regex

_\d+$ = look for underscore followed by any amount of digits [0-9]. $ denotes, only look for this at end of the string.

Then replace either of those if found with blank string ''

potential issue with replace("notify.")

I assume you also only want to strip this if it's at start of the string. I've added regex_replace in my code for this as well. This bug also exists in STABLE i assume and should also be fixed there.

other issue with all camera names are only integers

I originally had my camera's named in frigate to their last 3 digits of their IPs because I'm lazy. This caused errors where python thought they were int and not strings I believe, causing other issues. I didn't debug this and instead just changed them to be prefixed with a string. This issue is not resolved (or may not even exist)

diff

diff --git a/Frigate Camera Notifications/Beta b/Frigate Camera Notifications/Beta
index 2f77647..fb73870 100644
--- a/Frigate Camera Notifications/Beta	
+++ b/Frigate Camera Notifications/Beta	
@@ -679,7 +679,7 @@ blueprint:
 mode: parallel
 trigger_variables:
   input_camera: !input camera
-  camera: "{{ input_camera.rstrip('_1').rstrip('_2').rstrip('_3') | replace('camera.', '') }}"
+  camera: "{{ input_camera | regex_replace('^camera\.|_\d+$', '') }}"
   mqtt_topic: !input mqtt_topic
 trigger:
   - platform: event
@@ -694,7 +694,7 @@ trigger:
     id: frigate-event
 variables:
   input_camera: !input camera
-  camera: "{{ input_camera | replace('camera.', '') }}"
+  camera: "{{ input_camera | regex_replace('^camera\.', '') }}"
   camera_name: "{{ camera | replace('_', ' ') | title }}"
   input_base_url: !input base_url
   base_url: "{{ input_base_url.rstrip('/')}}"
@@ -705,7 +705,7 @@ variables:
   update_thumbnail: !input update_thumbnail
   ios_live_view: !input ios_live_view
   group: !input notify_group
-  group_target: "{{ group | lower | replace('notify.', '') | replace(' ','_') }}"
+  group_target: "{{ group | lower | regex_replace('^notify\.', '') | replace(' ','_') }}"
   zone_only: !input zone_filter
   input_zones: !input zones
   zones: "{{ input_zones | list | lower }}"

test code: test-strip.py

import re
# https://www.home-assistant.io/docs/configuration/templating/#regular-expressions
entity_prefix = 'camera.'
camera_name = 'outdoor.camera.3'
camera = entity_prefix + camera_name
print(camera)
# wrong, replaces 3 at end of string
print(camera.rstrip('_1').rstrip('_2').rstrip('_3'))
# ^ same code as above, but still wrong
print(camera.rstrip('_123'));

# also wrong, as this will replace "camera." anywhere in camera name, not just at beginning
print(camera.rstrip('_1').rstrip('_2').rstrip('_3').replace('camera.', ''))

# using regex. Checks for "camera." at the start of string.
# AND Checks for _[AnyAmountOfNumbers] at end of string and replaces with blank
print(re.sub('^camera\.|_\d+$', '', camera))

Version

dev

Automation Config

doesn't matter

Frigate Config

doesn't matter

Any other relevant information

No response

@suredesigntshirts suredesigntshirts added the bug Something isn't working label Jul 31, 2023
@SgtBatten
Copy link
Owner

SgtBatten commented Jul 31, 2023

Thanks very much for the feedback.

The argument you pass to rstrip in python is a set of characters to strip, not a string to strip

I was not aware of this so thank you. Ultimately I hope to remove this need altogether if Frigate implements an existing feature request to pass the camera name as an attribute.

replace "camera." bug

Agree that regex is the better way to implement these things as there is no doubt as to the accuracy. In terms of assessing the urgency of this change though. Can you have additional periods in an entity name? I'm not certain camera.outdoor.camera.321 is valid, which would mean the only place camera. and notify. can be is at the beginning of the string. I'll look into this.

To Do

  • Beta: replace rstrip with regex replace
  • Beta: replace replace('camera.', '') and replace('notify.', '') with regex replace
  • Stable: replace replace('camera.', '') and replace('notify.', '') with regex replace

@suredesigntshirts
Copy link
Author

I'm not certain camera.outdoor.camera.321 is valid

I don't know home assistant well enough to tell you this.

I'm just thinking of this issue from a string parsing frame.

Only reason I found this out, is because I ran into this issue and wanted to fix it. So figured I'd pass the info along.

@SgtBatten
Copy link
Owner

@suredesigntshirts how do you have this working? it is not compatible with HA

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants