Skip to content

Fix #62#63

Open
rajkundu wants to merge 1 commit intoSciKit-Surgery:masterfrom
rajkundu:master
Open

Fix #62#63
rajkundu wants to merge 1 commit intoSciKit-Surgery:masterfrom
rajkundu:master

Conversation

@rajkundu
Copy link
Copy Markdown
Contributor

This PR fixes #62 by adding the parallel probe configuration option. When configuration['parallel probe'] = True, concurrent.futures is imported, and concurrent.futures.ProcessPoolExecutor is used to simultaneously probe serial ports. Rather than check ports sequentially, this option allows _get_serial_port_name to return as soon as an NDI device is found on any serial port.

So far, this change has only been tested on an Intel MacBook Pro running macOS 12.5.

@thompson318
Copy link
Copy Markdown
Collaborator

Thanks @rajkundu, this looks a useful addition in principle. Ideally I'd like to see the change accompanied with a unit test so we don't drop code coverage. That should also help us no if it runs OK on Linux and Windows when it goes trough github actions. Do you think you can have a go at implementing that?

@rajkundu
Copy link
Copy Markdown
Contributor Author

I haven't written any before, but I have always wanted to learn, so sure - I'll try my best!

@thompson318
Copy link
Copy Markdown
Collaborator

OK, thanks. Please have a try and I'll be happy to help. You should be able to turn on github actions on your fork so you can see the test results each time you push.
Testing for this library is a little tricky as we don't have access to a continuous integration machine connected to an nditracker, so we make extensive use of Python's mock library. That enables us to create a mock version of ndicapy that returns what we need to test. I'd suggest you start with the file tests/test_sksurgerynditracker_mockndi_polaris.py, copy it and modify the settings to add "parallel probe", and see what happens.

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.

Simultaneously Probe Serial Ports for NDI devices

2 participants