search

lsgunth / pyft232

Python bindings to d2xx and libftdi to access FT232 chips with the same interface as pyserial
GNU Lesser General Public License v2.1
11 stars 11 forks source link

Skip devices without serial number in list_devices #18

Closed richey-v closed 5 years ago

richey-v commented 5 years ago

Underlying usb code raises ValueError if serial number is not available; ignore that exception in addition to just usb.USBError.

lsgunth commented 5 years ago

Can you please add the exception you are seeing in the commit message?

Also, FYI, you can force push a commit to the same branch and it will update this PR. You don't have to close and create a new PR every time you want to update your changes.

Thanks

richey-v commented 5 years ago

Force push is generally blocked for master on Github, right? I guess that's another argument for creating a topic branch. Point taken, though; I haven't done too many PRs here on Github.

richey-v commented 5 years ago

I'll see if I can dig up the original error... error occurred on a headless Raspberry Pi where pyft232 was finding a board serial device and I'm not sure I still have access to that machine.

richey-v commented 5 years ago

do you want that error in the commit or shall I create an Issue to report the error and link the PR to the Issue?

lsgunth commented 5 years ago

No, you can set master to block force pushes, but that's not the default. But you're also not force pushing to a master branch, you're pushing to a topic branch.

It's less work for everyone if you just force push to your altfix branch, then the changes appear here and all you'd have to do then is post a comment or something saying you're ready for another look.

You should really test that your change does something as well. If you can't easily reproduce the error and show that your change fixes it, then it makes me nervous merging such a commit.

Put the error in the commit message please. No need to create another issue and then it's there if I'm looking in 'git blame'.

richey-v commented 5 years ago

Ah the block force push to master is the Gitlab-CE default.

Github web is acting weird... I did the force push and didn't see anything on my fork when reloading, even after a few minutes. Then, went to My Repositories and I see the change.

richey-v commented 5 years ago

Looks like I made another Gitlab assumption on Github... the private Gitlab that I use regularly is configured to display Markdown just about everywhere. Looks like Github (or at least this repository) doesn't support Markdown in commit messages. Alas.

lsgunth commented 5 years ago

Yeah, you really shouldn't use markdown in commit messages regardless, that's not really a good idea seeing most people use git directly.

lsgunth commented 5 years ago

If you can drop the markdown quotes from the commit message I'll merge this right away.

richey-v commented 5 years ago

done. would you also be able to make a release here and on PyPi?

richey-v commented 5 years ago

I appreciate all of your rapid responses! I was using pyftdi for a good while but prefer this. pyftdi required heavy wizardry involving libusb.

lsgunth commented 5 years ago

Sure thing. I can't do the release today, but I'll try to do it next week.

lsgunth commented 5 years ago

The release is done on PyPi.

richey-v commented 5 years ago

Thanks!