improvement!: convert commands from Task-based to GenServer-based execution #27
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…cution Commands are now short-lived GenServers that can react to safety state changes and other messages during execution. Key changes: - `BB.Command` behaviour with `handle_command/3`, `result/1`, and `handle_safety_state_change/2` callbacks - `BB.Command.Server` GenServer wrapper managing lifecycle, param subscription, and result extraction - `BB.Command.ResultCache` ETS-based cache enabling `await/2` to retrieve results even after command process terminates - `BB.Command.await/2` and `yield/2` use `GenServer.call` with cache fallback - Commands can control state transitions via `next_state` option in result - Updated Arm, Disarm, and MoveTo commands to new pattern BREAKING CHANGE: Command handlers must now implement `BB.Command` behaviour with `handle_command/3` and `result/1` callbacks instead of the previous `handle/2` pattern.
- Update DSL handler type to accept `{module, keyword_list}` tuple
- Add `imports: [BB.Dsl.ParamRef]` to command entity for `param()` function
- Fix bug in Command.Server passing robot_module instead of robot_state
- Add comprehensive tests for static and parameterised command options
The param resolution pattern was duplicated across Actuator.Server, Sensor.Server, Controller.Server, and Command.Server. This extracts the common logic into a shared module providing: - `resolve_and_subscribe/2` - resolve ParamRefs and subscribe to changes - `resolve/2` - resolve ParamRefs without subscribing - `subscribe/2` - subscribe to parameter change topics - `handle_change/4` - handle parameter change messages All four server modules now use this shared implementation, removing approximately 70 lines of duplicated code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
ParamRefresolutionBB.Server.ParamResolutionChanges
New modules
BB.Command.Server- GenServer wrapper for command callback modulesBB.Command.ResultCache- ETS-based cache for command results (handles race between process death and await)BB.Error.State.CommandCrashed- Structured error for crashed commandsBB.Server.ParamResolution- Shared param resolution logic extracted from all server modulesUpdated modules
BB.Command- Full behaviour with__using__macro, new callbacks (handle_command/3,result/1,handle_safety_state_change/2,handle_options/2)BB.Robot.Runtime- Start GenServer instead of Task, handle completion via castBB.Supervisor- Add DynamicSupervisor for commandsBB.Dsl- Accept{module, opts}tuple format for command handlersArm,Disarm,MoveTo) updated to new patternActuator.Server,Sensor.Server,Controller.Server- Use sharedParamResolutionDocumentation
05-commands.mdtutorial for GenServer-based commandsAGENTS.mdBreaking Changes
BB.Commandbehaviour with new callbackshandle_command/3replaces the oldexecute/2callbackresult/1to extract the result from stateTest plan