Add onChange() method to V3 Trigger - #9359
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an onChange() convenience binding to the commandsv3 Trigger API, aligning it with existing trigger binding patterns (e.g., onTrue()/onFalse()) and providing a shorthand for scheduling a command on either edge.
Changes:
- Added
Trigger#onChange(Command)to schedule a command on both rising and falling edges. - Added Javadoc for the new
onChange()method.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /** | ||
| * Starts the command when the condition changes. | ||
| * | ||
| * @param command the command to start | ||
| * @return this trigger, so calls can be chained | ||
| */ |
| public Trigger onChange(Command command) { | ||
| requireNonNullParam(command, "command", "onChange"); | ||
| addBinding(BindingType.SCHEDULE_ON_RISING_EDGE, command); | ||
| addBinding(BindingType.SCHEDULE_ON_FALLING_EDGE, command); | ||
| return this; | ||
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f5fa46b3a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| addBinding(BindingType.SCHEDULE_ON_RISING_EDGE, command); | ||
| addBinding(BindingType.SCHEDULE_ON_FALLING_EDGE, command); |
There was a problem hiding this comment.
Skip the initial sample in onChange
Because m_previousSignal is null until the trigger is first polled, poll() classifies the initial sample as either a rising or falling edge. Registering the command for both edge types therefore guarantees that onChange() schedules it on the first Scheduler.run() even when the supplier has never changed, contrary to the method contract and the existing V2 onChange() behavior. This needs a dedicated binding or baseline handling that ignores the initial sample.
Useful? React with 👍 / 👎.
No description provided.