-
-
Notifications
You must be signed in to change notification settings - Fork 32
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 friendlyName to PortInfo interface <superseeded - will delete upon confirmation of alternative fix> #105
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Gareth Hancock <[email protected]>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -61,6 +61,7 @@ interface PortInfo { | |||
locationId: string | undefined; | |||
productId: string | undefined; | |||
vendorId: string | undefined; | |||
friendlyName?: string | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is technically true 🤔
Should we make it always present on other platforms but undefined to be consistent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I was trying to work out if we had set a precedent elsewhere... and the only example I could find was the LowLatency mode for Linux.... however upon a closer look, while it is only defined within the Linux specific interface definition, Windows actually returns False. So I think the PortStatus definition might need to be changed too :-(
Happy to pivot in either direction, but I think it would be good to be consistent in how we handle things across the package... Can you think of any other platform specific items we might need to consider at the same time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would assume that eventually we can provide the friendly name on other systems. So I wouldn't consider it platform specific by design.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I'll draft an alternative change that includes friendlyname within port info, and lowlatency within get info, but allows them to be undefined
I've proposed an alternative set of changes to cover this and allow for the fact that lowLatency is returned as False when getting port info on Mac and Win. This resulted in changes to bindings-interface, bindings-cpp, bindings-mock, and the website... I don't think it requires any changes to the node-serialport repository since we have allowed undefined. Please let me know if I've missed anything, or if you would like to suggest any additional changes ps - while I added lowLatency to the Get PortInfo for all platforms, I've left the Set Port Options as it was (linux specific) |
Update documentation per fix for serialport/node-serialport#2600
Adds
friendlyName
(Windows specific property) to Port definition