search

Villhellm / lovelace-animated-background

Animated backgrounds for lovelace
191 stars 60 forks source link

Bug report: Configuration options overriding each other #38

Open xBelladonna opened 3 years ago

xBelladonna commented 3 years ago

First of all I'd like to thank you for releasing this stellar frontend integration! It's made a huge aesthetic difference to my dashboard and I very much appreciate your work on this project.

Describe the bug It appears there is a race condition governing whether or not the included/excluded devices will prevail over the included/excluded users and vice versa. If using both included users and excluded devices configuration options as I do in my config, sometimes the excluded devices will not display the background but most times they do. Testing this with an excluded device and included user produces the same results as testing with an excluded user and included device. Removing one or the other configuration option resolves the problem.

EDIT: upon further testing it appears that explicitly setting the enabled bool to true will override both of these configuration options, i.e. no matter if the user and device both match a condition for exclusion, the animated background will appear regardless. This does not occur vice versa, i.e. if set to false, any included users/devices will not cause such an override.

Your Animated Background configuration

animated_background:
  enabled: true
  debug: true
  default_url: 'https://cdn.flixel.com/flixel/v26zyfd6yf0r33s46vpe.hd.mp4'
  included_users:
    - xBelladonna
  excluded_devices:
    - windows
    - android
  entity: sensor.sun_weather
  state_url:
    <huge map of sensor states and video url lists>

Version Numbers

Browser console log Console logs report device exclusion however animated background is still observed:

Animated Background DEBUG: Current device is excluded
Animated Background DEBUG: Removing view background for configuration: 
Object { enabled: true, debug: true, default_url: "https://cdn.flixel.com/flixel/v26zyfd6yf0r33s46vpe.hd.mp4", included_users: (1) […], excluded_devices: (2) […], entity: "sensor.sun_weather", state_url: {…} }
animated-background.js:44:15
Animated Background DEBUG: Starting, Debug mode enabled 
Animated Background DEBUG: Configured entity sensor.sun_weather is now above_horizon-cloudy 
true animated-background.js:44:15
Animated Background DEBUG: Current device is excluded
Animated Background DEBUG: Starting, Debug mode enabled 
Animated Background DEBUG: Configured entity sensor.sun_weather is now above_horizon-cloudy 
true animated-background.js:44:15
Animated Background DEBUG: Current device is excluded
Animated Background DEBUG: Starting, Debug mode enabled 
Animated Background DEBUG: Configured entity sensor.sun_weather is now above_horizon-cloudy 
true animated-background.js:44:15
Animated Background DEBUG: Current device is excluded
Animated Background DEBUG: Starting, Debug mode enabled 
Animated Background DEBUG: Configured entity sensor.sun_weather is now above_horizon-cloudy 
true animated-background.js:44:15
Animated Background DEBUG: Current device is excluded
Animated Background DEBUG: Removing view background for configuration: 
Object { enabled: true, debug: true, default_url: "https://cdn.flixel.com/flixel/v26zyfd6yf0r33s46vpe.hd.mp4", included_users: (1) […], excluded_devices: (2) […], entity: "sensor.sun_weather", state_url: {…} }

To Reproduce Steps to reproduce the behavior:

  1. Configure both an included/excluded users and included/excluded devices section
  2. Set the configuration such that the current device/user will be included by one of these options (either by device or by user, either one works) and excluded by another, either by explicit or implicit inclusion/exclusion
  3. Clear browser cache and refresh page to ensure new settings are loaded
  4. Observe that background is still loaded (most of the time) despite browser console reporting exclusion of device/user

Expected behavior It is expected that if a user/device matches a condition to be excluded, it will not be included by another configuration option, and vice versa, i.e. if a user has been explicitly excluded (or not explicitly included) then no matter if they are using an included (or not explicitly excluded) device, they will not observe an animated background, and if the user is included (or not explicitly excluded) but is using a device which has been explicitly excluded (or not explicitly included), they will also not observe an animated background.

It is also expected that the enabled option, if explicitly set to true, does not override the animated background display if a device/user has matched a condition for exclusion.

Device (please complete the following information):

Villhellm commented 3 years ago

Includes override excludes. That's the way the logic has been designed. Something has to take priority. https://github.com/Villhellm/lovelace-animated-background/blob/master/dist/animated-background.js#L230 this is where the definitions start for the inclusion/exclusion rules. If you can come up with a more logical pattern I would be more than happy to look at a PR