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

Add config_manager and utils commands #1231

Merged
merged 9 commits into from
Aug 15, 2024

Conversation

marcus-j-davies
Copy link
Member

@marcus-j-davies marcus-j-davies commented Jun 29, 2024

@AlCalzone , @raman325

This PR exposes the lookupDevice method of the ConfigManager class.

I think this should be added, for the following reason.

Often, after scanning a QR Code, front ends might want to display the Device Information, present in the config.

  • Label
  • Manufatcure
  • Description

Generally, specifics about the device, not usually found in the QR its self.
I also use this method, during a Smart Start operation, to advise the user the device is unknown, as the return is either a DeviceConfig or undefined

Example:

Screenshot 2024-06-29 at 17 12 44

You could argue, why not include this by calling lookupDevice during QR Code parsing within the server?
But to allow better 1:1 mapping of the API - I didn't, feel free to suggest otherwise.

Now, I am not too familiar with this code base, so let me know if something is not right with my changes.

@raman325
Copy link
Collaborator

I wonder if this should be in a separate config namespace? (so config.lookup_device instead of utils.lookup_device). I know it's a lot to add a new namespace just for a single command but this might be a good place to put other config related methods

@raman325
Copy link
Collaborator

or maybe config_manager

@AlCalzone
Copy link
Member

I vote for utils.. The config manager isn't going to get any more extensions, so you're looking at 2 methods total (lookup manufacturer by ID and device by fingerprint).

@raman325
Copy link
Collaborator

@marcus-j-davies spoke with Al about this. I added all of the APIs for utils and config manager to be consistent. Sorry for the delayed activity on this but we should be good to merge soon!

src/lib/config_manager/command.ts Show resolved Hide resolved
src/lib/utils/command.ts Outdated Show resolved Hide resolved
src/lib/utils/command.ts Outdated Show resolved Hide resolved
src/lib/utils/command.ts Show resolved Hide resolved
Copy link
Member

@AlCalzone AlCalzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small clarification

README.md Outdated Show resolved Hide resolved
@raman325 raman325 merged commit 6e8a6a1 into zwave-js:master Aug 15, 2024
1 check passed
@raman325 raman325 changed the title Add utils.lookup_device Add config_manager and utils commands Sep 19, 2024
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.

3 participants