Skip to content

hwPortUtils.listComPorts: Don't barf when SPDRP_FRIENDLYNAME is invalid#6462

Merged
jcsteh merged 2 commits into
masterfrom
comPortNoFriendly
Nov 3, 2016
Merged

hwPortUtils.listComPorts: Don't barf when SPDRP_FRIENDLYNAME is invalid#6462
jcsteh merged 2 commits into
masterfrom
comPortNoFriendly

Conversation

@jcsteh

@jcsteh jcsteh commented Oct 14, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixed another rare issue when scanning for serial ports on some systems which made some braille display drivers unusable.

hwPortUtils.listComPorts: In some rare cases, the SPDRP_FRIENDLYNAME registry property doesn't exist/isn't valid. In these cases, just use the port name as the friendly name.
Originally reported by @joshknnd1982 in #6007 (comment)

While we're at it:

  • Also use PortName for FriendlyName on ERROR_INSUFFICIENT_BUFFER. We haven't seen it, but callers depend on FriendlyName, so we want to fall back if it does happen.
  • For extra safety, skip the port altogether if PortName is empty.

@jcsteh
Fixed another rare issue when scanning for serial ports on some syste…
a24d178 
…ms which made some braille display drivers unusable.

hwPortUtils.listComPorts: In some rare cases, the SPDRP_FRIENDLYNAME registry property doesn't exist/isn't valid. In these cases, just use the port name as the friendly name.
Re #6007.
@jcsteh

jcsteh commented Oct 14, 2016

Copy link
Copy Markdown
Contributor Author

@feerrenrut, mind taking a peek?

feerrenrut
feerrenrut approved these changes Oct 14, 2016
View reviewed changes

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I added a couple of questions, I'll leave it up to you.

Comment thread source/hwPortUtils.py Outdated
@@ -231,7 +231,9 @@ def __str__(self):
):
# Ignore ERROR_INSUFFICIENT_BUFFER
if ctypes.GetLastError() != ERROR_INSUFFICIENT_BUFFER:

@feerrenrut feerrenrut Oct 14, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unrelated to your change. While it might be unlikely I think it would be a good idea to log an insufficient buffer error.

Comment thread source/hwPortUtils.py Outdated
raise ctypes.WinError()
# #6007: SPDRP_FRIENDLYNAME sometimes doesn't exist/isn't valid.
log.debugWarning("Couldn't get SPDRP_FRIENDLYNAME for %s: %s" % (port, ctypes.WinError()))
entry["friendlyName"] = port

@feerrenrut feerrenrut Oct 14, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could port be an empty string? Would that matter?

@joshknnd1982

joshknnd1982 commented Oct 14, 2016

Copy link
Copy Markdown

I'm so glad I was able to help you developer guys out with fixing NVDA
by providing error logs! I hope I can contribute more, in this way, in
the future! its fun! thanks for NVDA!

On 10/14/2016 12:16 AM, James Teh wrote:

Fixed another rare issue when scanning for serial ports on some
systems which made some braille display drivers unusable.

hwPortUtils.listComPorts: In some rare cases, the SPDRP_FRIENDLYNAME
registry property doesn't exist/isn't valid. In these cases, just use
the port name as the friendly name.
Originally reported by @joshknnd1982 https://github.com/joshknnd1982
in #6007 (comment)
#6007 (comment)

    You can view, comment on, or merge this pull request online at:

#6462

    Commit Summary

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#6462, or mute the thread
https://github.com/notifications/unsubscribe-auth/APPbfZlOSuXVKSHJemMXTP1ton0HkBMDks5qzwItgaJpZM4KWn19.

mozilla thunderbird email client

@jcsteh
hwPortUtils.listComPorts: Handle PortName being empty just in case. F…
52057df 
…or FriendlyName, log a warning and fall back to PortName for ERROR_INSUFFICIENT_BUFFER too.
@jcsteh

jcsteh commented Oct 18, 2016

Copy link
Copy Markdown
Contributor Author

@feerrenrut, I realised that we depend on friendlyName and it wasn't being set at all on ERROR_INSUFFICIENT_BUFFER. So, we now just treat ERROR_INSUFFICIENT_BUFFER the same as any other error for friendlyName. Also, just in case, I handle port being empty. Can you please take another look? Ta!

jcsteh added a commit that referenced this pull request Oct 20, 2016
@jcsteh
Incubates #6462
aafca09
@nvaccessAuto nvaccessAuto assigned jcsteh and unassigned feerrenrut Oct 20, 2016
@jcsteh jcsteh merged commit dbdc303 into master Nov 3, 2016
@nvaccessAuto nvaccessAuto added this to the 2016.4 milestone Nov 3, 2016
jcsteh added a commit that referenced this pull request Nov 3, 2016
@jcsteh
Update What's New for #6462.
280b5c4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@feerrenrut feerrenrut feerrenrut approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jcsteh jcsteh

Labels

None yet

Projects

None yet

Milestone

2016.4

Development

Successfully merging this pull request may close these issues.

4 participants

@jcsteh @joshknnd1982 @feerrenrut @nvaccessAuto