Skip to content

added parallel node for BT#141

Merged
MarcoLm993 merged 14 commits intomainfrom
feat/parallel_node
Mar 20, 2026
Merged

added parallel node for BT#141
MarcoLm993 merged 14 commits intomainfrom
feat/parallel_node

Conversation

@ste93
Copy link
Copy Markdown
Member

@ste93 ste93 commented Oct 28, 2025 •
edited
Loading

added parallel node for BT, needed for UC3

@ste93 ste93 force-pushed the feat/parallel_node branch from dcf942b to 0147311 Compare February 4, 2026 09:54
@ste93 ste93 self-assigned this Mar 4, 2026
@ste93 ste93 requested review from EnricoGhiorzi and ct2034 March 5, 2026 09:35
ste93 added 5 commits March 5, 2026 11:04
@ste93
added parallel node for BT
adc8092 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93
added also tests
b012777 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93
updated newline at end of file
c9b5a57 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93
updated file to match roaml format
2e0a861 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93
updated pre-commit fixes
3aa4aa4 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93 ste93 force-pushed the feat/parallel_node branch from ed9c0bb to 3aa4aa4 Compare March 5, 2026 10:04
ste93 added 5 commits March 5, 2026 15:22
@ste93
updated ascxml
76c3b95 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93
fixed parallel node
fe66e2d 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93
updated
29db83e 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93
test should now work, need to add the running part
b5f1737 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93
pre commit run
2270bc7 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93 ste93 marked this pull request as ready for review March 6, 2026 10:33
@ste93
Copy link
Copy Markdown
Member Author

ste93 commented Mar 6, 2026

@ct2034 @EnricoGhiorzi I've added the parallel node (needed for UC3) can you check this PR and if it is good we can merge it

@EnricoGhiorzi
Copy link
Copy Markdown
Contributor

EnricoGhiorzi commented Mar 7, 2026 •
edited
Loading

I tried pulling the PR and building the UC3 model with a parallel node but I see no such nodes in the generated files. Is this to be expected? Is there a way to have a compiled parallel node to test?

PS: I managed to compile the bt with the parallel node (I had to uncomment some stuff in NotifyUserComponent so the model might not work as intended). Generation of the node works well but verification still shows that the battery alarm is never raised, though I am not sure whether that depends on the Parallel node.

PPS: I had forgot to change the property to include the BatteryAlarmSkill which is now included in the model. This allowed me to verify that the battery properties actually run fine! The robot still does not charge its battery, but that was to be expected I believe. Anyway, at least in this test, the parallel node seems to be working.

EnricoGhiorzi
EnricoGhiorzi requested changes Mar 9, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@EnricoGhiorzi EnricoGhiorzi left a comment

Choose a reason for hiding this comment

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

Thank you @ste93 for the PR. I think the parallel node is mostly working, also based on empirical sperimentation, but there are a few points in the code (marked with relevant comments) that I think could be improved before we merge this.

Comment thread src/as2fm/resources/bt_control_nodes/parallel.ascxml Outdated
Comment thread src/as2fm/resources/bt_control_nodes/parallel.ascxml
Comment thread src/as2fm/resources/bt_control_nodes/parallel.ascxml Outdated
Comment thread src/as2fm/resources/bt_control_nodes/parallel.ascxml Outdated
Comment thread src/as2fm/resources/bt_control_nodes/parallel.ascxml Outdated
Comment thread src/as2fm/resources/bt_control_nodes/parallel.ascxml Outdated
Comment thread test/jani_generator/_test_data/bt_test_models/bt_test_parallel_failure.xml Outdated
Comment thread test/jani_generator/_test_data/bt_test_models/bt_test_parallel_success.xml Outdated
Comment thread test/jani_generator/_test_data/bt_test_models/main_test_parallel_failure.xml
Comment thread test/jani_generator/_test_data/bt_test_models/main_test_parallel_running.xml
@EnricoGhiorzi EnricoGhiorzi mentioned this pull request Mar 9, 2026
Open
@ste93
addressed changes requested in PR
9c9df78 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93
Copy link
Copy Markdown
Member Author

ste93 commented Mar 11, 2026

@EnricoGhiorzi I should have addressed most of the comments

ct2034
ct2034 reviewed Mar 12, 2026
View reviewed changes
Comment thread test/jani_generator/test_systemtest_behavior_tree_scxml.py
Comment thread test/jani_generator/_test_data/bt_test_models/main_test_parallel_running.xml
ct2034
ct2034 requested changes Mar 12, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ct2034 ct2034 left a comment

Choose a reason for hiding this comment

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

Thanks Stefano. This looks really good. The tests worked for me.

I just noticed that you also created main_test_parallel_running.xml but not the bt for it. I think that would be worth testing, too.

MarcoLm993
MarcoLm993 reviewed Mar 17, 2026
View reviewed changes
Comment thread test/jani_generator/_test_data/bt_test_models/bt_test_status_action.ascxml
@ste93
addressed comments from pr
bfd8a6f 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93
Copy link
Copy Markdown
Member Author

ste93 commented Mar 18, 2026

@MarcoLm993 @EnricoGhiorzi @ct2034 addressed the comments :)

@ste93
addressed proe commit changes
594f109 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
@ste93 ste93 requested review from EnricoGhiorzi and ct2034 March 18, 2026 09:37
@ste93 ste93 requested a review from MarcoLm993 March 18, 2026 09:37
@ste93
added missing file
e40f24a 
Signed-off-by: Stefano Bernagozzi <stefano.bernagozzi@iit.it>
MarcoLm993
MarcoLm993 approved these changes Mar 20, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@MarcoLm993 MarcoLm993 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks! :)

@MarcoLm993 MarcoLm993 merged commit 45e764c into main Mar 20, 2026
7 checks passed
@MarcoLm993 MarcoLm993 deleted the feat/parallel_node branch March 20, 2026 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MarcoLm993 MarcoLm993 MarcoLm993 approved these changes

@ct2034 ct2034 Awaiting requested review from ct2034

@EnricoGhiorzi EnricoGhiorzi Awaiting requested review from EnricoGhiorzi

Assignees

@ste93 ste93

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ste93 @EnricoGhiorzi @ct2034 @MarcoLm993