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

Picoprobe: fix for uid being specified with no available Picoprobes. #1117

Merged
merged 1 commit into from
Mar 10, 2021

Conversation

flit
Copy link
Member

@flit flit commented Mar 9, 2021

Fixes #1116

@flit
Copy link
Member Author

flit commented Mar 9, 2021

cc @newbrain I sent you an invitation to be a collaborator so you can review the code.

@flit flit force-pushed the bugfix/picoprobe_uid branch from 9a04490 to 7972e85 Compare March 9, 2021 22:08
@flit
Copy link
Member Author

flit commented Mar 9, 2021

Weird. After I forced pushed a little fix, the checks didn't run. I'll have to add some other change to see if it runs with another added commit.

@flit flit force-pushed the bugfix/picoprobe_uid branch from 7972e85 to 5548e26 Compare March 9, 2021 23:07
@flit
Copy link
Member Author

flit commented Mar 9, 2021

Force pushing triggered the checks this time. 🤷🏽

@flit flit requested a review from newbrain March 9, 2021 23:09
Copy link
Collaborator

@newbrain newbrain left a comment

Choose a reason for hiding this comment

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

Looks good to me.
I had in the meantime written very similar code (apart from the typing -> List[]), but I like this more.
The idea of returning different types (as in the original, a list or a single object) was not that good to begin with.

@@ -101,13 +102,10 @@ def close(self):
self._rd_ep = None

@classmethod
def enumerate_picoprobes(cls, uid=None):
def enumerate_picoprobes(cls, uid=None) -> List["PicoLink"]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I must admit this was new to me. Oh well, more PEPs to read...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. It's all pretty new to me, too. 😄 As long as I've been using python, I haven't been able to use any of the Python 3 features because Python 2 compatibility was required in everything I worked on. So I'm going to start trying to add some type annotations to pyocd to better document the APIs.

And if you think type annotations are fun, check out PEP 638. 😁

@flit flit merged commit f940a93 into pyocd:master Mar 10, 2021
@flit flit deleted the bugfix/picoprobe_uid branch March 13, 2021 19:51
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.

Picoprobe: exception raised when UID is specified but no Picoprobe is connected
2 participants