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

Issue 60 #61

Merged
merged 4 commits into from
Feb 23, 2024
Merged

Issue 60 #61

merged 4 commits into from
Feb 23, 2024

Conversation

Tinigriffy
Copy link
Contributor

Fix for Luci device tracker: AttributeError: 'list' object has no attribute 'upper' #60
It seems that openwrt can now send a list or a string and this is not handled. Fix it.

It seems that openwrt can now send a list or a string. I do not know if we could have multiple mac in the list. We simply pick up the first one.
@jstrot
Copy link

jstrot commented Feb 4, 2024

How about always converting the x['mac'] to a list and then doing mac in string_to_list(x['mac']) ?
That way an entry with multiple macs can be matched.

@Tinigriffy
Copy link
Contributor Author

I did it this way because I did not understood what the structure could return and how the lamba will behave :). I was wondering if it should not be reported as bug to openwrt. From my perspective, I would expect that the api always returns a list. So I took the simplest approach to fix the issue for me :)
But I totally agree this is not the most robust way to handle this case. May be something like str(x['mac']).upper().find(mac) could work in all cases.

@Tinigriffy
Copy link
Contributor Author

... May be something like str(x['mac']).upper().find(mac) could work in all cases.

I tried this new approach on the new HA version 2024.2.0 and it seems to work

@mrchi mrchi merged commit a87f67e into fbradyirl:master Feb 23, 2024
1 of 3 checks passed
@mexx42
Copy link

mexx42 commented Mar 2, 2024

i think we need a new version number so that it can be included in forthcoming ha releases ... ?

@mrchi
Copy link
Collaborator

mrchi commented Mar 4, 2024

i think we need a new version number so that it can be included in forthcoming ha releases ... ?

Yes, I've just fixed the test case that caused the CI failure and released version 1.1.17. Feel free to try it out and provide feedback.

@mexx42
Copy link

mexx42 commented Mar 6, 2024

Thanks, see home-assistant/core#107319 (comment)

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

Successfully merging this pull request may close these issues.

5 participants