Skip to content

Conversation

@jsiegle
Copy link
Member

@jsiegle jsiegle commented Nov 6, 2025

@jsiegle jsiegle requested review from anjaldoshi and Copilot November 6, 2025 23:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a callable stop() method to the EventListener class, allowing programmatic stopping of the event listener from another thread instead of relying solely on ctrl-C. The version is incremented to 1.0.1.

  • Added a running flag to control the event listener loop
  • Implemented a stop() method to set the flag to False
  • Enhanced error handling with a generic exception catch block

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/open_ephys/streaming/event_listener.py Added running flag, stop() method, and improved exception handling
src/open_ephys/streaming/README.md Updated documentation to mention the new stop() method
src/open_ephys/init.py Incremented version from 1.0.0 to 1.0.1

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 143 to 144
print("EventListener stopped.")
print("")
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 143-144 are incorrectly indented and will execute on every loop iteration instead of only when the loop exits. These lines should be dedented to align with the while statement so they execute after the loop completes.

Suggested change
print("EventListener stopped.")
print("")
print("EventListener stopped.")
print("")

Copilot uses AI. Check for mistakes.
print("Stopped by KeyboardInterrupt") # Add final newline
break
except Exception as e:
print(f"Error: {e}")
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generic exception handler catches and prints errors but doesn't break the loop or stop execution. This could mask critical errors and cause the listener to continue running in an invalid state. Consider whether certain exceptions should stop the listener or if the exception should be re-raised after logging.

Suggested change
print(f"Error: {e}")
print(f"Error: {e}")
raise

Copilot uses AI. Check for mistakes.
self.socket = self.context.socket(zmq.SUB)
self.socket.connect(self.url)
self.socket.setsockopt(zmq.SUBSCRIBE, b"")
self.running = False # Stop flag
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The running flag should be thread-safe since the documentation indicates stop() will be called from another thread. Consider using threading.Event or marking this as an atomic operation with appropriate synchronization to prevent race conditions.

Copilot uses AI. Check for mistakes.
@jsiegle jsiegle merged commit 4032c29 into main Nov 7, 2025
1 check passed
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.

4 participants