Skip to content

Feature/app slam#233

Merged
jonpg01 merged 7 commits into
feature/ros2_slam_nav_intfrom
feature/APPSlam
Jul 7, 2026
Merged

Feature/app slam#233
jonpg01 merged 7 commits into
feature/ros2_slam_nav_intfrom
feature/APPSlam

Conversation

@jonpg01

@jonpg01 jonpg01 commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@jonpg01 jonpg01 requested a review from stefanvogel June 24, 2026 20:51
Comment thread lib/APPSlam/src/App.h Outdated
Comment thread lib/APPSlam/src/App.cpp Outdated
Comment thread lib/APPSlam/src/SerialMuxChannels.h Outdated
Comment thread lib/APPSlam/src/App.h Outdated
Comment thread lib/APPSlam/src/App.h Outdated
Comment thread lib/APPSlam/src/App.cpp Outdated
Comment thread lib/APPSlam/src/App.h Outdated
Comment thread lib/APPSlam/src/App.h Outdated
Comment thread lib/APPSlam/src/App.h Outdated
Comment thread lib/APPSlam/src/App.h Outdated
Comment thread lib/APPSlam/src/App.cpp Outdated
Comment thread lib/APPSlam/src/App.cpp
Comment thread lib/APPSlam/src/App.cpp Outdated
Comment thread lib/APPSlam/src/App.cpp Outdated
Comment thread lib/APPSlam/src/App.cpp Outdated
@jonpg01 jonpg01 requested a review from stefanvogel July 2, 2026 08:36
Comment thread lib/APPSlam/src/App.cpp
Comment thread lib/APPSlam/src/App.cpp Outdated
Comment thread lib/APPSlam/src/App.cpp Outdated
#define CONFIG_LOG_SEVERITY (Logging::LOG_LEVEL_INFO)
#endif /* CONFIG_LOG_SEVERITY */

/** TCP server IP address if not defined by the build configuration. */

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should keep the old comment, now that you introduced

#ifndef APPSLAM_TCP_SERVER_IP

correctly. Same below for APPSLAM_TCP_SERVER_PORT

Comment thread lib/APPSlam/src/App.cpp
Comment thread lib/APPSlam/src/App.cpp Outdated
Comment thread lib/APPSlam/src/App.cpp
Comment thread lib/APPSlam/src/App.h Outdated
void loop();

/**
* Handle vehicle data received from SerialMuxProt and forward to ROS2 server.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doxygen @brief descriptions must use third-person singular present tense ("Connects", "Returns", "Handles"), not imperative ("Connect", "Return", "Handle").

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Applies to other places in this header of course as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Applied to all comments

@stefanvogel stefanvogel self-requested a review July 2, 2026 11:02
Comment thread lib/APPSlam/src/App.cpp Outdated
{
LOG_WARNING("Failed to serialize vehicle data.");
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread lib/APPSlam/src/App.h Outdated
void loop();

/**
* Handle vehicle data received from SerialMuxProt and forward to ROS2 server.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Applies to other places in this header of course as well.

@jonpg01 jonpg01 requested a review from stefanvogel July 3, 2026 07:54
@stefanvogel

Copy link
Copy Markdown
Collaborator

There are still 2 unresolved comments before I can review again...

Comment thread lib/APPSlam/src/App.h Outdated
@stefanvogel stefanvogel self-requested a review July 6, 2026 13:15
@jonpg01 jonpg01 merged commit b44a7ce into feature/ros2_slam_nav_int Jul 7, 2026
21 checks passed
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.

2 participants