Skip to content

Conversation

@shawnkchan
Copy link

@shawnkchan shawnkchan commented Oct 13, 2025

Summary

Raising this PR to get feedback on my implementation for the order execution logic. This PR covers Order execution - Order Parsing and Order execution - Node Sequencing.

This PR contains the following features:

  1. OrderManager class
    • Receives an Order message from the Order topic
    • Validates the order
    • Exposes API for OrderExecutor to sequence through the nodes and edges of the Order
  2. GraphValidator class
    • Checks that the graph in an Order is valid
  3. OrderGraphElement class
    • Parent class to Node and Edge class
  4. Node class
  5. Edge class
  6. Order Class
  7. Unit tests for OrderManager and GraphValidator

NOTE: IStateManager.hpp is a temporary class for mocking the StateManger in the unit tests.

@shawnkchan shawnkchan requested a review from sauk2 October 13, 2025 07:10
@shawnkchan shawnkchan changed the title Feature: Order Execution feat(order execution): adds OrderManager Oct 17, 2025
@shawnkchan shawnkchan changed the title feat(order execution): adds OrderManager feat(order_execution): adds OrderManager Oct 17, 2025
@sauk2
Copy link
Member

sauk2 commented Nov 19, 2025

Thanks for all the great work on this!
This is a solid iteration and the direction looks good. To make it easier to review, test and merge this safely, I would like to suggest breaking the work into smaller pieces. This will make it easier to validate each part and keep the main stable.

Some high-level feedback:

  • Retarget this PR to develop instead as we are holding off on pushing larger changes directly to main.

  • Let’s merge PR feat(types): create C++ structs for VDA5050 2.0.0 spec #6 first as it introduces hard types that both the OrderManager and StateManager can use. Getting those in first will reduce a lot of duplication.

  • Split the order and state logic into separate branches. Right now, this PR mixes order-related code with state-related code. To keep responsibilities clear,

    • OrderManager should focus strictly on order handling.
    • StateManager should focus strictly on state tracking.
    • The execution layer should serve as the bridge between the two.

    This separation would help us avoid having order-related classes directly mutate state and it keeps the architecture more testable.

@shawnkchan shawnkchan changed the base branch from main to develop November 20, 2025 02:27
shawnkchan and others added 13 commits November 20, 2025 11:18
* fix: export fmt as a dependency for downstream packages

Signed-off-by: Saurabh Kamat <[email protected]>

* feat: add vda5050 structs for internal use

Signed-off-by: Saurabh Kamat <[email protected]>

* feat: overload equality/inequality operators

Signed-off-by: Saurabh Kamat <[email protected]>

* feat: add action.hpp

Signed-off-by: Shawn Chan <[email protected]>

* feat: add action_parameter.hpp

Signed-off-by: Shawn Chan <[email protected]>

* feat: add action_parameter_factsheet.hpp

Signed-off-by: Shawn Chan <[email protected]>

* feat: add action_parameter_value.hpp

Signed-off-by: Shawn Chan <[email protected]>

* feat: add action_parameter_value_type.hpp

Signed-off-by: Shawn Chan <[email protected]>

* feat: add action_scope.hpp

Signed-off-by: Shawn Chan <[email protected]>

* feat: add blocking_type.hpp

Signed-off-by: Shawn Chan <[email protected]>

* feat: add agv_action.hpp

Signed-off-by: Shawn Chan <[email protected]>

* feat: add value_data_type.hpp

Signed-off-by: Shawn Chan <[email protected]>

* feat: add remaining structs for vda5050 messages

Signed-off-by: Shawn Chan <[email protected]>

* fix: change action param to have key-value pairs instead of custom type

Signed-off-by: Saurabh Kamat <[email protected]>

* fix: remove optional from action array

Signed-off-by: Saurabh Kamat <[email protected]>

* fix: remove deleted include

Signed-off-by: Saurabh Kamat <[email protected]>

* Update vda5050_types/include/vda5050_types/agv_action.hpp

* Update vda5050_types/include/vda5050_types/node.hpp

* Update vda5050_types/include/vda5050_types/node.hpp

---------

Signed-off-by: Saurabh Kamat <[email protected]>
Signed-off-by: Shawn Chan <[email protected]>
Co-authored-by: Shawn Chan <[email protected]>
Co-authored-by: Shawn Chan <[email protected]>
Signed-off-by: Shawn Chan <[email protected]>
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