-
Notifications
You must be signed in to change notification settings - Fork 39
refactor+fix: add LangChainAgent
abstraction and fix HRI communication
#538
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
Conversation
ReActAgent
LangChainAgent
abstraction
LangChainAgent
abstraction LangChainAgent
abstraction and fix HRI communication
LangChainAgent
abstraction and fix HRI communicationLangChainAgent
abstraction and fix HRI communication
baebccb
to
60f4348
Compare
@CodeRabbit full review |
WalkthroughThis set of changes refactors the implementation of the Changes
Sequence Diagram(s)sequenceDiagram
participant Human
participant ROS2HRIConnector
participant LangChainAgent
participant LangChainRunnable
participant HRICallbackHandler
Human->>ROS2HRIConnector: Send HRI message (/from_human)
ROS2HRIConnector->>LangChainAgent: source_callback(msg)
LangChainAgent->>LangChainAgent: Buffer message, possibly interrupt
LangChainAgent->>LangChainAgent: Reduce messages (per behavior)
LangChainAgent->>LangChainRunnable: stream(aggregated message, callbacks)
LangChainRunnable-->>LangChainAgent: Streaming response
LangChainAgent->>HRICallbackHandler: Callback with response
HRICallbackHandler->>ROS2HRIConnector: build_message, send_message (/to_human)
ROS2HRIConnector->>Human: Human receives response
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🧹 Nitpick comments (2)
tests/agents/test_langchain_agent.py (1)
20-20
: Import suggests a type annotation issue.The
newMessageBehaviorType
appears to be imported fromrai.agents.langchain.agent
, suggesting it might be a type alias or enum, but it's using Python's lower camel case naming convention which is unusual for a type. Consider using snake_case or PascalCase for consistency with Python conventions.src/rai_core/rai/agents/langchain/callback.py (1)
50-52
: Fix typo in logging message.There's a typo in the logging message: "targer" should be "target".
- self.logger.info(f"Sending {len(tokens)} tokens to targer: {target}") + self.logger.info(f"Sending {len(tokens)} tokens to target: {target}")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
examples/agents/react.py
(1 hunks)src/rai_core/rai/agents/__init__.py
(1 hunks)src/rai_core/rai/agents/langchain/agent.py
(1 hunks)src/rai_core/rai/agents/langchain/callback.py
(3 hunks)src/rai_core/rai/agents/langchain/react_agent.py
(1 hunks)src/rai_core/rai/agents/react_agent.py
(0 hunks)src/rai_core/rai/communication/hri_connector.py
(1 hunks)src/rai_core/rai/communication/ros2/connectors/base.py
(1 hunks)src/rai_core/rai/communication/ros2/connectors/hri_connector.py
(1 hunks)src/rai_core/rai/communication/ros2/messages.py
(3 hunks)tests/agents/test_langchain_agent.py
(1 hunks)
💤 Files with no reviewable changes (1)
- src/rai_core/rai/agents/react_agent.py
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/rai_core/rai/agents/__init__.py (1)
src/rai_core/rai/agents/langchain/react_agent.py (1)
ReActAgent
(26-42)
src/rai_core/rai/communication/ros2/connectors/base.py (2)
src/rai_core/rai/communication/ros2/connectors/hri_connector.py (1)
general_callback_preprocessor
(56-57)src/rai_core/rai/communication/base_connector.py (1)
general_callback_preprocessor
(173-175)
src/rai_core/rai/agents/langchain/callback.py (2)
src/rai_core/rai/communication/hri_connector.py (3)
HRIConnector
(140-153)HRIMessage
(38-134)build_message
(146-153)src/rai_core/rai/communication/ros2/connectors/hri_connector.py (1)
send_message
(39-54)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-test-ros2 (humble)
- GitHub Check: build-and-test-ros2 (jazzy)
🔇 Additional comments (22)
src/rai_core/rai/agents/__init__.py (1)
17-17
: Import path updated to reflect new LangChainAgent abstractionThe import path for
ReActAgent
has been updated to reflect its new location in thelangchain
subpackage, which aligns with the PR objective of extracting theLangChainAgent
abstraction.src/rai_core/rai/communication/ros2/connectors/base.py (1)
155-155
: Added proper return type annotationAdding the return type annotation
-> T
improves type safety and makes the API more explicit. This correctly indicates that the method returns an instance of the generic typeT
that the connector is bound to.src/rai_core/rai/communication/hri_connector.py (1)
146-146
: Method renamed from private to public APIThe method has been renamed from
_build_message
tobuild_message
, removing the leading underscore to indicate it's now part of the public API. This aligns with the PR objective of updating HRI communication to use theregister_callback
mechanism.This change is needed as the method is likely now being called from the refactored agent implementations.
src/rai_core/rai/communication/ros2/messages.py (2)
15-15
: Improved module import robustnessThe conditional import of
rai_interfaces.msg
usingimportlib.util.find_spec
is a better approach than the previous implementation. It checks for the module's presence before attempting to import it and provides a more accurate warning message.Also applies to: 31-33
68-76
: Enhanced subclassing support with cls instead of concrete classUsing
cls
instead of the concrete class nameROS2HRIMessage
in the return statement of the class method supports proper inheritance and follows the Liskov Substitution Principle. This change allows subclasses to correctly extend this method without unexpected behavior.tests/agents/test_langchain_agent.py (1)
22-31
: Test parametrization looks good with comprehensive behavior coverage.The parametrization covers various message reduction behaviors effectively. I particularly appreciate the inclusion of both normal and interrupt-based behaviors.
examples/agents/react.py (4)
16-20
: Import reorganization looks good.The imports have been correctly updated to reflect the new module structure with the refactored
ReActAgent
now imported fromrai.agents.langchain.react_agent
.
26-27
: ROS2HRIConnector instantiation simplified.The connector is now instantiated without explicit parameters, aligning with the refactored connector that no longer manages topic configurations directly.
28-34
: Agent configuration updated to use target_connectors.The agent now properly uses the
target_connectors
parameter mapping the/to_human
topic to thehri_connector
instance, aligning with the new LangChainAgent base class design.
34-35
: Source subscription improved with explicit method.Using
subscribe_source
makes the subscription more explicit and aligns with the new agent design that supports multiple connectors with explicit subscription methods.src/rai_core/rai/agents/langchain/callback.py (3)
17-17
: Type import update is appropriate.Adding
Dict
to the imports alongsideList
andOptional
is a good practice for proper type annotations.
30-30
: Type annotation update for connectors parameter.The type annotation change from
dict
toDict
aligns with PEP 585 recommendations for using the standard typing module annotations.
50-64
: Message sending approach updated for new connector interface.The refactored code now correctly:
- Iterates through target-connector pairs
- Builds a message using
connector.build_message
- Sends it with
connector.send_message(to_send, target)
This aligns with the simplified connector interface in the refactored architecture.
src/rai_core/rai/agents/langchain/react_agent.py (2)
26-42
: Well-structured ReActAgent implementation.The
ReActAgent
class extendsLangChainAgent
and provides a clean interface for constructing React agents with LangChain models and tools. The implementation:
- Accepts appropriate parameters (target connectors, language model, tools, state, system prompt)
- Creates a runnable with
create_react_runnable
- Properly initializes the parent class
This approach promotes code reuse by leveraging the shared functionality in the base class while maintaining specific React agent behavior.
26-42
:✅ Verification successful
Verify implementation of create_react_runnable function.
Make sure that the
create_react_runnable
function properly initializes a React agent runnable with the provided parameters. Since it's a key component connecting the agent to the LangChain ecosystem, it's important to ensure it's working correctly.
🏁 Script executed:
#!/bin/bash # Check the implementation of create_react_runnable function fd "create_react_runnable" --type file | xargs catLength of output: 50
To locate and inspect the actual implementation of
create_react_runnable
, let’s do a recursive text search across the repo:
🏁 Script executed:
#!/bin/bash # Search for both the definition and all call sites of create_react_runnable rg -n "create_react_runnable" -A5 -B5 .Length of output: 5789
To inspect the full implementation of
create_react_runnable
, let’s pull its definition and body:
🏁 Script executed:
#!/bin/bash # Show definition and body of create_react_runnable rg -n "def create_react_runnable" -A200 src/rai_core/rai/agents/langchain/runnables.pyLength of output: 1683
Implementation Verified:
create_react_runnable
correctly initializes the React agent runnableThe function
- Defaults to the configured “complex_model” if no LLM is provided
- Builds a
StateGraph
with proper start → LLM and optional tools → LLM transitions- Binds tools to the LLM when supplied
- Compiles and returns the runnable graph
No further changes are needed.
src/rai_core/rai/communication/ros2/connectors/hri_connector.py (3)
39-54
: Well-structured message publishing method with clear parametersThe refactored
send_message
method provides a clean interface for publishing HRI messages with appropriate optional parameters for QoS profile customization.
56-57
: Good addition of preprocessor for human messagesThe new callback preprocessor method correctly handles incoming messages and sets the author to "human", which supports the HRI communication flow.
32-32
: Well-defined inheritance structureThe connector now properly inherits from both
ROS2BaseConnector
andHRIConnector
, establishing a clear type hierarchy and interface implementation.src/rai_core/rai/agents/langchain/agent.py (4)
147-166
: Well-implemented message reduction behaviorThe static method for applying different message reduction behaviors is well-structured with clear logic for each behavior type.
32-43
: Good type definitions with clear documentationThe code provides well-defined types for state and message behaviors, which improves API clarity (aside from the typo mentioned earlier).
90-96
: Clean thread management for agent executionThe run method properly creates and starts a dedicated thread for the agent's execution loop.
137-146
: Robust agent stopping mechanismThe stop method ensures proper cleanup of threads and events, preventing resource leaks.
src/rai_core/rai/communication/ros2/connectors/hri_connector.py
Outdated
Show resolved
Hide resolved
- rollback registering using __call__ method - add subscribe_source method
46e3a19
to
ca72444
Compare
a43ddf9
to
bc6a7cb
Compare
@CodeRabbit full review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/agents/test_langchain_agent.py (1)
1-42
: Test implementation for the new message reduction behaviors is good, but has typos in behavior names.The test properly verifies the
_apply_reduction_behavior
static method of theLangChainAgent
class using appropriate parameterization for different message behaviors.There are two typos in the behavior names:
- ("interupt_take_all", [1, 2, 3], [], [1, 2, 3]), - ("interupt_keep_last", [1, 2, 3], [], [3]), + ("interrupt_take_all", [1, 2, 3], [], [1, 2, 3]), + ("interrupt_keep_last", [1, 2, 3], [], [3]),examples/agents/react.py (1)
29-31
: Updated agent construction with the new API pattern.The
target_connectors
parameter is now used instead of the previousconnectors
dictionary, which is consistent with the refactoredReActAgent
implementation.There's a small typo in the comment: "agnet's" should be "agent's".
- }, # agnet's output is sent to /to_human ros2 topic + }, # agent's output is sent to /to_human ros2 topicsrc/rai_core/rai/agents/langchain/agent.py (1)
182-187
: Consider using event-based waiting instead of sleepThe use of
time.sleep(0.5)
could be replaced with an event-based approach to be more efficient and responsive to new messages.def _run_agent(self): if len(self._received_messages) == 0: self._agent_ready_event.set() self.logger.info("Waiting for messages...") - time.sleep(0.5) + # Wait for a signal that messages have arrived + message_wait_event = threading.Event() + self._message_wait_event = message_wait_event + message_wait_event.wait(timeout=0.5) + self._message_wait_event = None returnWith corresponding updates to the message receiving code to signal this event.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (13)
examples/agents/react.py
(1 hunks)examples/agents/react_ros2.py
(1 hunks)src/rai_core/rai/agents/__init__.py
(1 hunks)src/rai_core/rai/agents/langchain/agent.py
(1 hunks)src/rai_core/rai/agents/langchain/callback.py
(3 hunks)src/rai_core/rai/agents/langchain/react_agent.py
(1 hunks)src/rai_core/rai/agents/react_agent.py
(0 hunks)src/rai_core/rai/communication/hri_connector.py
(1 hunks)src/rai_core/rai/communication/ros2/connectors/base.py
(1 hunks)src/rai_core/rai/communication/ros2/connectors/hri_connector.py
(1 hunks)src/rai_core/rai/communication/ros2/messages.py
(3 hunks)tests/agents/test_langchain_agent.py
(1 hunks)tests/communication/ros2/test_connectors.py
(1 hunks)
💤 Files with no reviewable changes (1)
- src/rai_core/rai/agents/react_agent.py
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/rai_core/rai/agents/__init__.py (1)
src/rai_core/rai/agents/langchain/react_agent.py (1)
ReActAgent
(26-42)
src/rai_core/rai/communication/ros2/connectors/base.py (2)
src/rai_core/rai/communication/ros2/connectors/hri_connector.py (1)
general_callback_preprocessor
(79-80)src/rai_core/rai/communication/base_connector.py (1)
general_callback_preprocessor
(173-175)
examples/agents/react_ros2.py (8)
src/rai_core/rai/agents/runner.py (1)
AgentRunner
(65-115)src/rai_core/rai/agents/langchain/react_agent.py (1)
ReActAgent
(26-42)src/rai_core/rai/communication/ros2/connectors/ros2_connector.py (1)
ROS2Connector
(19-20)src/rai_core/rai/communication/ros2/context.py (1)
ROS2Context
(23-146)src/rai_core/rai/communication/ros2/connectors/hri_connector.py (1)
ROS2HRIConnector
(32-80)src/rai_core/rai/tools/ros2/generic/toolkit.py (1)
ROS2Toolkit
(22-50)examples/agents/react.py (1)
main
(24-37)src/rai_core/rai/agents/langchain/agent.py (2)
subscribe_source
(136-140)run
(155-161)
src/rai_core/rai/agents/langchain/react_agent.py (3)
src/rai_core/rai/agents/langchain/runnables.py (2)
create_react_runnable
(69-114)ReActAgentState
(29-38)src/rai_core/rai/agents/langchain/agent.py (1)
LangChainAgent
(45-257)src/rai_core/rai/communication/hri_connector.py (2)
HRIConnector
(140-153)HRIMessage
(38-134)
src/rai_core/rai/communication/ros2/connectors/hri_connector.py (5)
src/rai_core/rai/communication/hri_connector.py (1)
HRIConnector
(140-153)src/rai_core/rai/communication/ros2/connectors/base.py (4)
ROS2BaseConnector
(44-293)send_message
(137-153)register_callback
(158-178)general_callback_preprocessor
(155-156)src/rai_core/rai/communication/ros2/messages.py (3)
ROS2HRIMessage
(45-108)to_ros2_dict
(78-104)from_ros2
(47-76)src/rai_core/rai/communication/base_connector.py (3)
send_message
(85-94)register_callback
(109-133)general_callback_preprocessor
(173-175)src/rai_core/rai/communication/ros2/api/topic.py (1)
publish
(153-180)
🔇 Additional comments (39)
src/rai_core/rai/communication/ros2/messages.py (2)
31-34
: Improved conditional ROS2 dependency handlingThe conditional importing using
importlib.util.find_spec
is a more elegant approach than the previous try-except block. This early detection of missing dependencies provides clearer messaging to users when ROS2 message functionality isn't available.
68-68
: Good polymorphism enhancementChanging the return statement to use
cls
instead of explicitly returning an instance ofROS2HRIMessage
enables proper subclassing. This is an important improvement that follows the factory method pattern and ensures that subclasses ofROS2HRIMessage
can use this method correctly.src/rai_core/rai/agents/__init__.py (1)
17-17
: LGTM: Updated import path for refactored ReActAgentThe import path has been correctly updated to match the new location of the
ReActAgent
class, which now inherits from the newLangChainAgent
base class. This is a necessary change for the architectural refactoring described in the PR.tests/communication/ros2/test_connectors.py (1)
173-173
: LGTM: Updated connector instantiationThe
ROS2HRIConnector
is now instantiated without explicit topic targets, aligning with the refactored implementation that no longer requires explicit topic configuration. The test still performs the same verification of message publishing and reception functionality.src/rai_core/rai/communication/ros2/connectors/base.py (1)
155-155
: LGTM: Added return type annotationAdding the explicit return type annotation
-> T
to thegeneral_callback_preprocessor
method improves type safety and aligns with the implementation in the subclassROS2HRIConnector
. This makes the code more maintainable and helps catch potential type errors.src/rai_core/rai/communication/hri_connector.py (1)
146-146
: Method visibility change from protected to public.The method
_build_message
has been renamed tobuild_message
, changing its visibility from protected to public. This change aligns with the refactoring of agent architecture where the newLangChainAgent
and derived classes directly use this method instead of accessing a protected method.examples/agents/react_ros2.py (2)
15-21
: Clean imports aligned with the new architecture.The imports correctly reference the new module structure following the refactoring of
ReActAgent
into therai.agents.langchain
package.
23-38
: Good example demonstrating the new API pattern.This example properly demonstrates:
- The use of the refactored
ReActAgent
with the newtarget_connectors
parameter- The simplified
ROS2HRIConnector
instantiation without explicit topic configuration- The new subscription pattern using
subscribe_source
which leverages theregister_callback
method- The use of
AgentRunner
for lifecycle managementThis provides a clear reference implementation for users to understand the updated agent lifecycle and communication patterns.
examples/agents/react.py (3)
16-19
: Updated imports to match the refactored architecture.The imports have been correctly updated to reference:
- The relocated
ReActAgent
class fromrai.agents.langchain.react_agent
- The explicit import of
HRIMessage
fromrai.communication.hri_connector
- The updated import path for
ROS2HRIConnector
These changes align with the refactoring performed in this PR.
26-26
: Simplified connector instantiation.The
ROS2HRIConnector
instantiation has been simplified by removing explicit topic configuration, which aligns with the updated connector implementation that now uses theregister_callback
method for topic subscription.
34-37
: Updated agent lifecycle management.The agent lifecycle is now managed directly through method calls (
run()
,wait()
,stop()
) rather than usingAgentRunner
. This reflects the new agent design that manages its own asynchronous execution and message handling internally.src/rai_core/rai/agents/langchain/callback.py (5)
17-17
: Updated type import for better consistencyThe import statement has been expanded to include
Dict
from typing, which aligns with Python typing conventions and the use of capitalized type names.
30-30
: Updated type annotation for better correctnessChanged from lowercase
dict
toDict
from the typing module, which is the correct typing convention.
50-51
: Improved variable naming and logging clarityRenamed variables from
(connector_name, connector)
to(target, connector)
which better represents their purpose and aligns with the rest of the API.
53-59
: Updated message handling approachThe previous implementation using
connector.send_all_targets
has been replaced with a more explicit two-step process:
- Building a message with
connector.build_message
- Sending it with
connector.send_message(to_send, target)
This change aligns with the updated interface in
HRIConnector
and provides better separation of concerns.
60-63
: Generalized log message referencesUpdated log messages to reference generic "hri_connector" or "target" instead of connector-specific names, making the logs more consistent and maintainable.
src/rai_core/rai/agents/langchain/react_agent.py (2)
15-24
: Well-structured imports with proper organizationThe imports are well-organized, separating standard library imports from third-party imports and internal modules, following good Python practices.
26-42
: Clean implementation of ReActAgent classThe
ReActAgent
class extendsLangChainAgent
and provides a clean implementation focused on React agent functionality. The constructor appropriately delegates to the superclass after creating a runnable with the provided LLM, tools, and system prompt.This design follows good OOP principles by reusing the base class functionality while specializing for a specific use case.
src/rai_core/rai/communication/ros2/connectors/hri_connector.py (6)
15-21
: Improved import organization and runtime checkingThe imports have been reorganized and a runtime check for
rai_interfaces.msg
has been added usingimportlib.util.find_spec
, which is a better approach than importing and handling import errors.
26-29
: Updated warning message for missing packageThe warning message now correctly references "rai_interfaces.msg" rather than "sounddevice", making it more relevant and helpful for users.
32-37
: Improved class definition with multiple inheritanceUpdated the class definition to inherit from both
ROS2BaseConnector[ROS2HRIMessage]
andHRIConnector[ROS2HRIMessage]
, which provides a more coherent interface. The constructor has been simplified by removing explicit targets and sources parameters.
39-54
: Streamlined send_message implementationThe
send_message
method now uses the generic_topic_api.publish
method with explicit message type and QoS parameters, replacing the previous approach. This makes the code cleaner and more maintainable.
56-77
: Added flexible register_callback methodAdded a new
register_callback
method that provides a flexible way to register callbacks for source topics with proper error handling and defaults. This improves the API by making it more intuitive and consistent with the base class.
79-80
: Added general_callback_preprocessor for message conversionAdded a new
general_callback_preprocessor
method that converts incoming ROS2 messages intoROS2HRIMessage
instances with a fixed message author, improving message handling.src/rai_core/rai/agents/langchain/agent.py (15)
15-30
: Well-structured imports and dependenciesThe imports are well-organized, grouped by standard library, third-party packages, and internal modules. The use of specific imports rather than wildcard imports improves code clarity.
32-34
: Simple BaseState TypedDict definitionThe
BaseState
TypedDict provides a clear structure for the agent's state, with a simple definition that includes a list of messages.
36-42
: Clear message behavior type definitionThe
newMessageBehaviorType
Literal type clearly defines the allowed values for message handling behavior, making the API more self-documenting and enabling type checking.
45-104
: Comprehensive docstring with examplesThe class docstring is thorough and well-structured, explaining:
- The purpose of the agent
- Its parameters with detailed descriptions
- How to use the agent with examples
- Different message behavior options
This makes the class easily understandable and usable.
106-134
: Robust initialization with proper thread safetyThe constructor initializes all necessary components including:
- Logger
- Agent runnable and state
- Callback handlers
- Thread-safe message buffer with lock
- Thread management primitives for concurrency
The use of a lock for the message buffer and various events for signaling between threads shows good attention to thread safety.
136-141
: Simple source subscription methodThe
subscribe_source
method provides a clean interface for subscribing to message sources using the connector'sregister_callback
method.
142-154
: Thread-safe message handling in callThe
__call__
method safely handles incoming messages by:
- Using a lock to protect buffer access
- Checking for buffer overflow and dropping oldest messages if needed
- Submitting interrupt tasks to an executor if needed
- Logging message reception
- Appending the message to the buffer
The thread safety and error handling here are well-implemented.
155-161
: Clean agent start methodThe
run
method checks if the agent is already running, creates and starts a new thread for the agent loop, sets the agent ready event, and logs the start. This prevents duplicate starts and ensures proper initialization.
163-170
: Agent readiness check and wait methodsThe
ready
andwait
methods provide ways to check if the agent is ready and to wait until it's ready and its message queue is empty. This enables proper synchronization in client code.
172-181
: Agent interruption handlingThe
_interrupt_agent_and_run
method handles interrupting the agent if it's processing a message, waits until it's ready again, and clears the interrupt event. This mechanism allows for responsive handling of high-priority messages.
182-204
: Main agent processing loopThe
_run_agent
method:
- Checks if there are messages to process
- Sets the agent ready status
- Reduces messages according to the configured behavior
- Converts messages to LangChain format
- Updates the agent state
- Runs the agent with callbacks
- Handles interruptions during processing
- Ensures the agent is marked as ready when done
The method is well-structured with proper error handling via try/finally.
205-209
: Efficient agent run loopThe
_run_loop
method runs a loop that periodically checks if the agent should stop and if it's ready to process the next message. The short wait time (0.01s) ensures responsiveness while avoiding busy-waiting.
210-219
: Clean agent stop implementationThe
stop
method:
- Sets the stop event
- Sets the interrupt event to stop any ongoing processing
- Waits for the agent to be ready
- Joins the thread if it exists
- Logs the agent stopping and stopped states
This ensures proper cleanup and prevents resource leaks.
220-238
: Clear message reduction behavior implementationThe
_apply_reduction_behavior
static method implements different message handling strategies based on the configured behavior:
- "take_all": processes all messages from oldest to newest
- "keep_last": takes only the most recent message
- "queue": processes one message at a time in FIFO order
The implementation is clear, well-commented, and raises appropriate errors for invalid behaviors.
240-257
: Thread-safe message reductionThe
_reduce_messages
method:
- Acquires the buffer lock to safely access the message buffer
- Applies the configured reduction behavior
- Combines messages into a single message with concatenated text and aggregated media
- Returns a new HRIMessage with the combined content
The thread safety and message handling are well-implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Purpose
With new API based on the register_callback, the previous implementation is obsolete. Also #536 broke the HRI communication in
ReActAgent
Proposed Changes
register_callback
ROS2HRIConnector
Issues
Testing
Summary by CodeRabbit
Summary by CodeRabbit