Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Typo/bug at convert_Graph.cpp (when entry_event is nullptr) #363

Closed
1 task done
kjchee opened this issue Jun 7, 2024 · 2 comments
Closed
1 task done

[Bug]: Typo/bug at convert_Graph.cpp (when entry_event is nullptr) #363

kjchee opened this issue Jun 7, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@kjchee
Copy link

kjchee commented Jun 7, 2024

Before proceeding, is there an existing issue or discussion for this?

OS and version

Ubuntu 22.04

Open-RMF installation type

Source build

Other Open-RMF installation methods

No response

Open-RMF version or commit hash

main

ROS distribution

Humble

ROS installation type

Source build

Other ROS installation methods

No response

Package or library, if applicable

No response

Description of the bug

  1. There is a typo/bug at convert_Graph.cpp line 909 : entry_event->execute(factory);

  2. The entry_event here should be exit_event.

  3. It crashed when entry_event is nullptr.

Steps to reproduce the bug

  1. in parse_graph.cpp, add a lane with exit event but without entry event, something like:

const YAML::Node& lanes = level.second["lanes"];
for (const auto& lane : lanes)
{
std::size_t begin = lane[0].as<std::size_t>();
std::size_t end = lane[1].as<std::size_t>();
rmf_utils::clone_ptr<Event> entry_event;
rmf_utils::clone_ptr<Event> exit_event;
exit_event = Event::make(Lane::Dock(dock_name, duration));
graph.add_lane( {begin, entry_event}, {end, exit_event});
}

  1. fleet adapter will crash at convert_Graph.cpp at line 909

  2. It crashed because the entry_event is nullptr.

  3. So, the entry_event here should be exit_event.

Expected behavior

No response

Actual behavior

No response

Additional information or screenshots

No response

@kjchee kjchee added the bug Something isn't working label Jun 7, 2024
@mxgrey
Copy link
Contributor

mxgrey commented Jun 7, 2024

Excellent catch! You're absolutely right and #364 has been opened to fix this.

Luckily these serialized nav graphs were only being transmitted to the nav graph visualizer, so the serialized entry/exit events were never being used and weren't risking any misbehavior except for this crash. On the other hand, the lack of misbehavior is why this went for so long without being noticed.

@luca-della-vedova
Copy link
Member

#364 merged, closing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants