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

Suggestion: Report when waiting for darkness #50

Closed
tschundler opened this issue Dec 16, 2024 · 7 comments · Fixed by #53
Closed

Suggestion: Report when waiting for darkness #50

tschundler opened this issue Dec 16, 2024 · 7 comments · Fixed by #53

Comments

@tschundler
Copy link

When I first ran this, it seemed stuck. I tried both Pixelblaze & WLED with no luck. So I dug into the code, and added some output to try to diagnose the problem, and quickly realized - it doesn't take a reference frame and a bright spot at the edge of my binds was confusing it.

Pending #33, I think it would help to let the user know when it is waiting for a dark frame. eg something like

--- a/marimapper/detector.py
+++ b/marimapper/detector.py
@@ -128,10 +128,13 @@ def enable_and_find_led(
     led = LED2D(led_id, view_id)

     if led_backend is None:
+        print("backend is unset")
         return led

     # First wait for no leds to be visible
     while find_led(cam, threshold, display) is not None:
+        print("Waiting for no LEDs to be visible. Check the preview. Does the environment need to be darker?")
+        time.sleep(0.1)
         pass

     # Set the led to on and start the clock
@TheMariday
Copy link
Owner

This is actually a duplicate of #32

However this is written better so keeping it!

This used to be a feature actually but it got removed in the v2 version.

Thank you for raising it!

@TheMariday
Copy link
Owner

The backend is none warning should be caught by this line in the scanner.py file:

 if len(self.led_id_range) == 0:
    print(
        "LED range is zero, have you chosen a backend with 'marimapper --backend'?"
    )
    continue

Adding a warning to the user that there are leds lit without over-warning them is going to be tricky. I'm going to adjust how the queues work that should make it a bit easier but yes, working on this but a bit harder than planned.

New queue system requests a range of leds instead of a stream of id's. This will also make #38 easier to integrate

@tschundler
Copy link
Author

Is over-warning really a problem? It is text on a CLI rather than an alert()
You could show the warning again only if some time has passed (eg max every few seconds)

@TheMariday
Copy link
Owner

I've got an idea as to how to implement this in a nice way, I'll keep you posted.

The reason it's tricky is just the structure of the code at the moment isn't conducive to this kind of warning, so it's time to refactor!

The only way the detector differentiates between being in detection mode or being in preview mode is whether the detection input queue has any elements in which isn't a good way to switch between two modes of operation.

I think I will move it to a more explicit switch so that we can add checks and background removal etc etc to the proecss.

Should make start / stop events easier to add.

@TheMariday TheMariday added this to the v2.3 milestone Dec 19, 2024
@TheMariday TheMariday mentioned this issue Dec 24, 2024
Merged
@TheMariday TheMariday linked a pull request Dec 24, 2024 that will close this issue
Merged
@tschundler
Copy link
Author

Testing out the change:


Start scan? [y/n]: y
Capturing sequence:   0%|                                                           | 0/289 [00:00<?, ?LEDs/s][ERROR/DetectorProcess-1] Failed to set autofocus to 0
[ERROR/DetectorProcess-1] Failed to set autofocus to 0
[ERROR/DetectorProcess-1] Failed to set focus to 0
[ERROR/DetectorProcess-1] Failed to set focus to 0
[ERROR/DetectorProcess-1] Failed to put camera into manual exposure mode 0
[ERROR/DetectorProcess-1] Failed to put camera into manual exposure mode 0
[ERROR/DetectorProcess-1] failed to set camera gain to 0
[ERROR/DetectorProcess-1] failed to set camera gain to 0
[ERROR/DetectorProcess-1] Failed to set exposure to -10
[ERROR/DetectorProcess-1] Failed to set exposure to -10
[ERROR/DetectorProcess-1] Detector process can detect an LED when no LEDs should be visible
[ERROR/DetectorProcess-1] Detector process can detect an LED when no LEDs should be visible
[ERROR/DetectorProcess-1] Failed to set autofocus to -1
[ERROR/DetectorProcess-1] Failed to set autofocus to -1
[ERROR/DetectorProcess-1] Failed to set focus to -1
[ERROR/DetectorProcess-1] Failed to set focus to -1
[ERROR/MainProcess] Scan failed
Capturing sequence:   0%|                                                           | 0/289 [00:03<?, ?LEDs/s]
Start scan? [y/n]:

it seems everything is logged twice. Also, the reason it didn't start is lost in messages about unable to set camera properties. (shouldn't it set those properties before looking for LEDs?)

I was actually confused the first time and though I didn't press "y" and maybe hit another letter instead that it was asking again to start.

@TheMariday
Copy link
Owner

That's really interesting! I haven't seen things log twice before like this! I'll have an investigate.

I'll also try and put the camera into dark mode prior to the scan starting to raise any of these errors early.

I'll also try and combine them into one or two lines so it's less overwhelming.

Glad the feature works though!

@TheMariday
Copy link
Owner

This should be much improved in v2.3 which can be grabbed with pipx upgrade marimapper

Happy to reopen this if the issue is still persisting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants