mKeRix / room-assistant

Presence tracking and more for automation on the room-level
https://www.room-assistant.io
MIT License
1.26k stars 122 forks source link

If item not on white list and black list not in use item still gets reported. #53

Closed gouldner closed 6 years ago

gouldner commented 6 years ago

Based on the logic of how white list and black list are handled. The white list seems useless to me. Here is my use case. I simply want to add ids of ble tokens I want to detect. So I populated white list with 2 ble token ids. Blacklist is left empty...I have a ton of devices in my home which send ble messages. So I don't want to have to black list them all and they when a new id walks into my house I don't want that reported either.

However the code says

if ((whitelist.length > 0 && whitelist.includes(id))
    || !(blacklist.length > 0 && blacklist.includes(id))) {
Send logic....
}

So as soon as black list is empty the second part of the or condition is always met. !(blacklist.length > 0 && blacklist.includes(id)) = always true because blacklist.length = 0

This essentially disables the white list. Even if I add a fake id to blacklist I get same result because the && condition looks for the token in the black list and it isn't found which returns false which in negated to = true (always) again.

Honestly I don't think it makes sense to have whitelist and blacklist. I would think you either use one or the other. Not sure what it should mean to have both.

So maybe a better approach would be to have two parameters use_list:{blacklist,whitelist,none} list:{{id},{id},...}

So if using whitelist it means send only ids found on list. If using blacklist then it means send ids not found on list.

gouldner commented 6 years ago

I decided to change my copy as follows. This lets white and blacklist work if populated. If populated together white list would take priority because an item must first be white listed. However if a white listed item was then black listed it would get excluded.
I don't really see a point in this unless you added wild carding or something like that.

Here is the logic I decided to implement.

    // RRG - Fix whitelist/blacklist logic
    var use_whitelist = whitelist.length > 0;
    var use_blacklist = blacklist.length > 0;

    // If white list not in use || id is found on white list then white list = true
    // If white list true (not in use or found) and blacklist not in use or id NOT blacklisted = true
    if ((!use_whitelist || whitelist.includes(id))
     && (!use_blacklist || !blacklist.includes(id))) {
        send message logic.....
    }
gouldner commented 6 years ago

I just sent you a pull request with this fix and one other for update frequency.

mKeRix commented 6 years ago

Closing, since you fixed this yourself. Thanks again! :)