Skip to content

Reload tree#61

Closed
daisukes wants to merge 2 commits intoBehaviorTree:masterfrom
CMU-cabot:reload-tree
Closed

Reload tree#61
daisukes wants to merge 2 commits intoBehaviorTree:masterfrom
CMU-cabot:reload-tree

Conversation

@daisukes
Copy link
Contributor

@daisukes daisukes commented Feb 18, 2020

I have been working with Groot and ROS Navigation2 (modified and publishing ZMQ logger) and had a problem with the monitoring view.

crash reproduction procedure

  1. createTreeFromFile and publish
  2. monitor (this is okay)
  3. second createTreeFromFile and publish, then crash

problem

It tries to find nodes matching to the UIDs in flatbuffer but it needs to know the new tree

fixed strategy

  1. check all UIDs in flatbuffer if monitor already know the UIDs
  2. if not, reload tree from the server and update status

@facontidavide facontidavide mentioned this pull request Feb 18, 2020
Closed
@facontidavide
Copy link
Contributor

facontidavide commented Feb 18, 2020

At first sight it lok goot (thanks a lot). I will find some time to double check and I will merge it soon.

@facontidavide facontidavide self-assigned this Feb 18, 2020
@naiveHobo
Copy link

naiveHobo commented Sep 2, 2020

This would be much needed addition for real-time monitoring. Any progress here?

This was referenced Sep 12, 2020
Closed
Merged
@gramss
Copy link
Contributor

gramss commented Sep 12, 2020

Hey there,

I was testing this PR with my PR for nav2.

Without this PR, I had a few errors with reloading trees crashing Groot, as described here.
But now, After some nasty manual testing sequences from me, I'm quite comfortable to say that this PR (+ my small fix for this PR) fixed all my problems regarding Groot crashing.

The only thing that is missing is using a different xml, like in this scenario:

  1. navigate with XML1 -> Goal reached
  2. navigate with XML2 -> Goal reached
  3. navigate with XML1 -> Goal reached

(all while the same Groot Monitor is active/open and connected)
Maybe @fmrico , as the expert of using different BT-XMLs, would be a good candidate to help here? :)

@fmrico
Copy link

fmrico commented Sep 15, 2020

Hi @gramss

Sorry, there isn't much I can contribute here :(

facontidavide pushed a commit that referenced this pull request Sep 19, 2020
merging #61
9f81708
@facontidavide
Copy link
Contributor

facontidavide commented Sep 19, 2020

sorry for ignoring this so long. Thanks for the contribution and the patience.

Merged manually because of the conflict.

@gramss gramss mentioned this pull request Sep 19, 2020
Merged
facontidavide pushed a commit that referenced this pull request Sep 28, 2020
@gramss
Small Fix for Reloading Tree from #61 (#96)
a3121bd 
* Add missing err:out_of_range code-sections + intent fixes

* Revert "Add missing err:out_of_range code-sections + intent fixes"

This reverts commit fa8a75b.

* indentation fixes

* indentation fixes - again

* get node status out of try/catch for compiler

* main thing: move rest of uid-based logic in try/catch for reload
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facontidavide facontidavide Awaiting requested review from facontidavide

Assignees

@facontidavide facontidavide

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@daisukes @facontidavide @naiveHobo @gramss @fmrico