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

Fix microscope installation guide error #382

Closed
wants to merge 2 commits into from

Conversation

talonchandler
Copy link
Collaborator

Fixes a small error @ieivanov and I spotted during a discussion of this work.

Currently, the next release is planned to include a single .dll USB device driver. We might consider distributing this device driver with MM if we get permission from Meadowlark, but this is currently a low priority.

@deprecated-napari-hub-preview-bot
Copy link

deprecated-napari-hub-preview-bot bot commented Jun 29, 2023

Preview page for your plugin is ready here:
https://preview.napari-hub.org/mehta-lab/recOrder/382
Updated: 2023-08-03T23:20:32.138869

@ieivanov
Copy link
Contributor

From reading micro-manager/mmCoreAndDevices#341 I see that the device adapter which ships with MM is built agains USB driver V108. That USB driver version do we distribute with the GitHub release? The device adapter instruction page says to use the .dll which Meadowlark distributes. Let's point our installation guide to the MM page so we can keep these instruction in one place. Here we could add instructions for legacy installations (if we are supporting those)?

@talonchandler
Copy link
Collaborator Author

I'm a bit confused about what we should do with the USB driver. Should we continue to distribute via recOrder (the plan that was in my head), or should we stop distributing and tell the user to use the version supplied by Meadowlark (the instructions on the device adapter page)? I'm not sure I know all of the history/permissions with distributing this file.

From reading micro-manager/mmCoreAndDevices#341 I see that the device adapter which ships with MM is built against USB driver V108. What USB driver version do we distribute with the GitHub release?

For recOrder<=0.3.0 we distribute an older USB driver (that works with MM 20210920), and I was planning for the recOrder==0.4.0 release to be distributed with usbdrvd.dll built against V108. Happy to change this plan.

The device adapter instruction page says to use the .dll which Meadowlark distributes. Let's point our installation guide to the MM page so we can keep these instruction in one place.

My (minor?) concern with doing this is that that instruction page isn't pinned to a specific release, so the instructions may change. My intent with these instructions is to keep them specific to the version they're distributed with, so that we have a hope of installing/recreating old versions.

Here we could add instructions for legacy installations (if we are supporting those)?

I think we can support legacy version via their legacy documentation. I don't think we should have installation instructions for legacy versions on the main branch.

@mattersoflight
Copy link
Member

It is great to see the MeadowlarkLC device adapter in the main MM repo!

My vote is to support only the current version of the device adapter/USB driver (that works with the current version of recOrder) while we are figuring out a software architecture amenable to broader deployment. Do you see a reason to support legacy installations?

@ieivanov
Copy link
Contributor

@talonchandler let's chat about this in person

@ziw-liu ziw-liu added bug Something isn't working documentation Improvements or additions to documentation labels Jul 24, 2023
@ziw-liu ziw-liu added this to the 0.4.0 milestone Jul 24, 2023
@talonchandler talonchandler self-assigned this Jul 27, 2023
@talonchandler
Copy link
Collaborator Author

talonchandler commented Jul 27, 2023

@ieivanov and I just had a chat about how to handle this:

  • We decided that when we release recOrder-napari==0.4.0 we will distribute the usbdrvd.dll built against V108 via the recOrder GitHub release page.
  • We'll link to this driver and instruct the user to do drag-and-drop it into their MM installation. We'll put the instructions in the recOrder docs. It doesn't seem like we'll need to link to the micromanager device driver instruction page.
  • We decided to keep installation instructions with their versions---no need to maintain legacy documentation. For example, 0.3.0 documentation will live with the 0.3.0 branch/release and 0.4.0 documentation will live with the 0.4.0 branch/release.

@ieivanov
Copy link
Contributor

ieivanov commented Aug 3, 2023

@talonchandler are you still working on this PR or is it ready to merge? Are you planning to add the dependency table here?

@talonchandler
Copy link
Collaborator Author

Thanks. I'm finishing up a couple things on this branch right now. I'll request your review on this branch (updated docs in preparation for 0.4.0) and #394 at the same time because I want to keep a couple things synchronized.

@talonchandler talonchandler changed the base branch from 0.4.0dev to main August 3, 2023 23:14
@talonchandler talonchandler changed the base branch from main to 0.4.0dev August 3, 2023 23:14
@talonchandler talonchandler changed the base branch from 0.4.0dev to main August 3, 2023 23:14
@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Merging #382 (a0a0cb9) into main (802cc62) will increase coverage by 5.26%.
Report is 4 commits behind head on main.
The diff coverage is 27.38%.

@@           Coverage Diff            @@
##            main    #382      +/-   ##
========================================
+ Coverage   1.75%   7.02%   +5.26%     
========================================
  Files         20      24       +4     
  Lines       4841    4429     -412     
========================================
+ Hits          85     311     +226     
+ Misses      4756    4118     -638     
Files Changed Coverage Δ
recOrder/acq/acquisition_workers.py 0.00% <0.00%> (ø)
recOrder/calib/Calibration.py 0.00% <ø> (ø)
recOrder/io/utils.py 0.00% <0.00%> (ø)
recOrder/plugin/main_widget.py 0.00% <0.00%> (ø)
recOrder/tests/cli_tests/test_cli.py 100.00% <100.00%> (ø)
recOrder/tests/cli_tests/test_settings.py 100.00% <100.00%> (ø)
recOrder/tests/conftest.py 100.00% <100.00%> (ø)
recOrder/tests/util_tests/test_overlays.py 100.00% <100.00%> (ø)

... and 7 files with indirect coverage changes

@talonchandler talonchandler changed the base branch from main to 0.4.0dev August 3, 2023 23:35
@talonchandler
Copy link
Collaborator Author

This PR was against 0.4.0dev, and I couldn't easily wrangle it into a form that would easily merge into main. I cherry-picked the key commits and opened a clear PR for discussion on #395.

Closing this.

@talonchandler talonchandler deleted the fix-docs-error branch August 5, 2023 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants