-
Notifications
You must be signed in to change notification settings - Fork 36
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
Log hardwareId of unknown devices at a lower verbosity level #237
Comments
@timjackson : I'm looking into this together with a few other issues. I get the idea, but could you specify what message you mean specifically and when a user would encounter this? Example: Fully unknown devices (#250) will actually cause the addon to fail, so logging is not the main issue here. Is that the use case you're trying to address or do you have another use case in mind? |
TL;DR: The "case" block in PlejdApi.js which enumerates certain hardwareIds as "-unknown-" is problematic and causes the problem described in this issue. Probably best to not include hardwareIds at all, unless it's actually known what devices they are. Long version: I realise in hindsight that this particular case was probably a bit "special", because hardwareId 14 at that time was enumerated in PlejdApi.js but as an "unknown" device, as opposed to not being listed at all. This had the advantage of making the device work, but the disadvantage of making it difficult to identify the hardwareId and report upstream what the device actually was. The solution discussed in #250 looks like it probably solves this issue, with the condition that "unknown" hardware IDs are not added to an "unknown" case in the "switch" block in PlejdApi.js. Currently on the main branch, hwID 15 and 16 are still in that "-unknown-" block which leads to this issue; I'd suggest they are thus removed entirely from that switch statement, although it looks from #250 that hwID 15 is now identified in any case, and hwID 16 is possibly non-existent. |
@timjackson: Completely agree with the "-unknown" things. The new branch below has an updated device type register and also removes all unknowns. https://github.com/icanos/hassio-plejd/tree/feature/new-devices-2022-10 Please report back if you have the possibility to test it. |
Closing this as being solved in the upcoming 0.10.0 release |
Addon version: 0.9.1
Proposal:
Log, at "info" or at most "debug" (NOT "verbose" or "silly") level, sufficient data for "regular" users to report unrecognised discovered devices to the project, without resorting to advanced debugging processes. At a minimum, this includes the "hardwareId", but could also include other relevant data identified through discovery, for example the "device.firmware.notes" field (which appears to contain the "consumer" device name, at least in some cases).
Alternative 1: Return the model of unknown devices as "
-unknown (hardwareId=<hardwareId>)-
" rather than "-unknown-
".Alternative 2: Return the model of unknown devices as "
<device.firmware.notes>
" rather than "-unknown-
", at least if device.firmware.notes is non-empty. Perhaps also with the hardwareId included.Background/rationale:
If discovered devices have a hardwareId which is not recognised, it is not easy for users, particularly those without development experience, to report to the project what type of hardware is discovered, and its corresponding hardwareId. Today, this information is only available with logging set to "silly" level, which is difficult or impossible to access from the Home Assistant UI due to the volume of log data. Obtaining the raw logs from the Home Assistant OS is non-trivial, time-consuming (at least the first time) and a large step up in difficulty for non-developers as it requires substantially more technical knowledge as well as jumping through several hoops.
Providing an easy way for non-technical/developer users to see and report the hardwareId of discovered but non-recognised devices will likely make it easier to add support for new devices and prevent issues like #232, #215, #230 etc. stalling.
The text was updated successfully, but these errors were encountered: