Conversation
Add real-time segmentation preview during image acquisition with robust synchronization to prevent race conditions and interference. Features: - Live segmentation with blur metrics and object detection preview - Real-time feedback during acquisition Fixes: - Prevent retained MQTT "Done" messages from skipping pump cycles - Tag acquisition pump commands with from_acquisition flag - Acquisition lock prevents UI pump controls from interfering - fsync ensures file is written before announcing via MQTT - File stability check before reading captured images
sonnyp
left a comment
sonnyp
left a comment
There was a problem hiding this comment.
This looks awesome.
I see a couple of improvements to be made but we can do that incrementally after it is merged.
The only things I would like us to fix before merging are
- regressions compared to what we have now
- potential API breaks for clients
Please add a segmenter/testlive.js file next to segmenter/test.js to trigger/test live segmentation.
|
|
||
| pump_started = False | ||
| # FIX: Track acquisition state to prevent UI commands from interrupting acquisition | ||
| acquisition_in_progress = False |
There was a problem hiding this comment.
This is something I wanted to have as well 👍
However it doesn't belong in the controller. controllers should only concern themselves with providing a simple hardware API.
Ideally we have a backend app handling "business logic". Until then we can do that logic in the node-red frontend. WDYT?
| # FIX: Only process Done if we are actually waiting for pump to finish | ||
| # This prevents retained/stale Done messages from triggering early return | ||
| if not self._waiting_for_pump: | ||
| loguru.logger.debug( | ||
| f"Ignoring pump Done (not waiting for pump): {self._mqtt.msg['payload']}" | ||
| ) | ||
| self._mqtt.read_message() | ||
| continue |
There was a problem hiding this comment.
I introduced the retained messages. Can you elaborate on what the issue is? Maybe with a sequence of pseudo events.
| # FIX: Use fsync on specific file to ensure write completes before MQTT publish | ||
| # os.sync() is system-wide and async - doesn't guarantee this file is written | ||
| with open(capture_path, "rb") as f: | ||
| os.fsync(f.fileno()) |
There was a problem hiding this comment.
| # FIX: Use fsync on specific file to ensure write completes before MQTT publish | |
| # os.sync() is system-wide and async - doesn't guarantee this file is written | |
| with open(capture_path, "rb") as f: | |
| os.fsync(f.fileno()) | |
| # Use fsync to ensure write completes before MQTT publish | |
| with open(capture_path, "rb") as f: | |
| os.fsync(f.fileno()) |
makes sense
I'm going to benchmark this as part of https://github.com/fairscope/PlanktoScope3/issues/375 to measure impact but I agree we should do this anyway
at least until we optimize sequence of events and start capturing n+1 before n finished writing
There was a problem hiding this comment.
os.fsync(f.fileno()) end us being slightly faster than os.sync
| # Copyright (C) 2021 Romain Bazile | ||
| # | ||
| # This file is part of the PlanktoScope software. | ||
| # | ||
| # PlanktoScope is free software: you can redistribute it and/or modify | ||
| # it under the terms of the GNU General Public License as published by | ||
| # the Free Software Foundation, either version 3 of the License, or | ||
| # (at your option) any later version. | ||
| # | ||
| # PlanktoScope is distributed in the hope that it will be useful, | ||
| # but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| # GNU General Public License for more details. | ||
| # | ||
| # You should have received a copy of the GNU General Public License | ||
| # along with PlanktoScope. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
There was a problem hiding this comment.
| # Copyright (C) 2021 Romain Bazile | |
| # | |
| # This file is part of the PlanktoScope software. | |
| # | |
| # PlanktoScope is free software: you can redistribute it and/or modify | |
| # it under the terms of the GNU General Public License as published by | |
| # the Free Software Foundation, either version 3 of the License, or | |
| # (at your option) any later version. | |
| # | |
| # PlanktoScope is distributed in the hope that it will be useful, | |
| # but WITHOUT ANY WARRANTY; without even the implied warranty of | |
| # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | |
| # GNU General Public License for more details. | |
| # | |
| # You should have received a copy of the GNU General Public License | |
| # along with PlanktoScope. If not, see <http://www.gnu.org/licenses/>. |
Copyright / license headers are not useful (also the attribution is wrong :D )
You retain copyright over what you write. The license is already specified globally.
| IMG_BASE = "/home/pi/data/img" | ||
| OBJECTS_BASE = "/home/pi/data/objects" |
There was a problem hiding this comment.
I recently introduced PLANKTOSCOPE_DATA_PATH to allow running the segmenter outside of a PlanktoScope
can you use the same logic as 1f84f2e#diff-059c3e4c6af9c25ca8f0562bb9887799bd0ffefe21b49f175fdd8c494d5bb0b6 ?
| # Paths for visualization output | ||
| IMG_BASE = "/home/pi/data/img" | ||
| OBJECTS_BASE = "/home/pi/data/objects" | ||
| LIVE_STATS_FILE = "/tmp/live_seg_stats.json" |
There was a problem hiding this comment.
please use https://docs.python.org/3/library/tempfile.html#tempfile.gettempdir instead of hardcoding /tmp
| # Hardware config path (same as used by controller) | ||
| HARDWARE_CONFIG_PATH = "/home/pi/PlanktoScope/hardware.json" |
There was a problem hiding this comment.
this will prevent the segmenter from being run outside of a PlanktoScope
It looks like this is only needed to read process_pixel_fixed.
Can the live segmenter use the same mechanism as the current segmenter instead?
We have process_min_ESD that is given by the frontend and we use scikit equivalent_diameter_area
Search for __process_min_ESD in https://github.com/PlanktoScope/PlanktoScope/blob/cf9169bb302c686dffc5aa8b5c2883c2e08766b9/segmenter/planktoscope/segmenter/__init__.py
ESD is not an accurate name for it (since we don't process spherical objects) but I kept it for backward compatibility. We can rename it later.
| "className": "", | ||
| "x": 800, | ||
| "y": 180, | ||
| "y": 200, |
There was a problem hiding this comment.
please revert unrelated changes (flows.json)
sonnyp
changed the title
3 participants
Summary
Add real-time segmentation preview during acquisition with fixes for pump synchronization issues.
Features
Bug Fixes