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

Fix Code Style On james-vogt/update-ros2 #2271

Open
wants to merge 8 commits into
base: ros2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake")
# Because we use ninja, we have to explicitly turn on color output for the compiler
if("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fcolor-diagnostics -Werror=return-stack-address -Werror=switch")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fuse-ld=lld")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fuse-ld=lld-16")
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -fuse-ld=lld")
else()
message(WARNING "You are using GCC; prefer to use clang if it is installed with the flags `-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++`.")
Expand Down
49 changes: 49 additions & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
Important changes were made when upgrading the stack to ROS2 Humble. A list
of changes and the reasoning behind them is listed below:

As of Nov 19, 2023:
1. The makefile was changed to work with the newer version of CMake
that installs by default on Ubuntu 22.04. In particular, the
cmake commands no longer need the --target flag. The CMAKE_PREFIX_PATH
variable at the beginning of the file has also been changed.

############## This change is important ###############
2. install/setup.bash - This has not been changed, but it is important
to note that distutils is deprecated and slated for removal in
Python 3.12. There are no direct replacements for distutils, so
a ticket should be opened ASAP to fix this. However, there is no
immediate issue with leaving it as is because Ubuntu 22.04 comes with
Python 3.10 by default. Do note that this is also the case in
install/setup.zsh.

3. source.bash - Source commands now reference humble and Ubuntu 22.04
instead of foxy and Ubuntu 20.04.

4. rj_common/testing/rj_common_convert_test.cpp - the rclcpp::Duration
class no longer accepts a single integer argument for milliseconds.
Updated a line referencing this outdated constructor to use
a std::chrono::milliseconds instead.

5. rj_common/include/rj_common/time.hpp - Changed for similar reasons to (4)

6. rj_utils/src/logging.cpp - RCLCPP_DEBUG and similar macros accept
C strings now; updated calls to these macros

7. rj_config/CMakeLists.txt - added find_package(fmt), this change complements
change number 6.

8. soccer/src/soccer/strategy/agent/agent_action_client.cpp and hpp -
Updated lines 211 to 219 to use lambda expressions instead of std::bind,
Changed a method to take in a different parameter type.
Previously took a future template holding a GoalHandleRobotMove::SharedPtr,
Now just takes in the GoalHandleRobotMove::SharedPtr.

9. soccer/src/soccer/ui/field_view.cpp - Added a preprocessor directive
to include QPainterPath, which has been separated into its own namespace.
Qt5 likely has other changes as well

10. soccer/src/soccer/ui/robot_status_widget.hpp - Added a preprocessor
directive to include the std::optional namespace.

This changelog should be updated to reflect any further changes completed
before this upgrade is fully adopted.
4 changes: 4 additions & 0 deletions install/setup.bash
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ _pythonpath_add() {
fi
}

# NOTE: Distutils is deprecated and scheduled to be removed in Python3.12.
# No direct replacement for the command exists, and in Ubuntu 22.04,
# this script will produce a warning about this. We may need to find a
# replacement library in the future.
_PYTHON_LIB_PATH=$(python3 -c "from distutils.sysconfig import get_python_lib; print(get_python_lib(prefix='${_INSTALL_PATH}'))")
_pythonpath_add "${_PYTHON_LIB_PATH}"
unset _PYTHON_LIB_PATH
Expand Down
8 changes: 4 additions & 4 deletions makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ SHELL=/bin/bash -o pipefail
MAKE_FLAGS = --no-print-directory
TESTS = *
FIRMWR_TESTS = -i2c -io-expander -fpga -piezo -neopixel -attiny -led -radio-sender -radio-receiver
export CMAKE_PREFIX_PATH=/opt/ros/foxy
export CMAKE_PREFIX_PATH=/opt/ros/humble

# circleci has 2 cores, but advertises 32, which causes OOMs
ifeq ($(CIRCLECI), true)
Expand All @@ -20,17 +20,17 @@ CMAKE_FLAGS=-DCMAKE_INSTALL_PREFIX="$(shell pwd)/install" -DNO_WALL=ON -DCMAKE_C
# usage: $(call cmake_build_target, target, extraCmakeFlags)
define cmake_build_target
mkdir -p build-debug
cd build-debug && cmake -GNinja -Wno-dev -DNO_WALL=ON -DCMAKE_BUILD_TYPE=Debug $(DEBUG_FLAGS) $(CMAKE_FLAGS) --target -DBUILD_TESTS=ON .. && ninja $(NINJA_FLAGS) $1 install
cd build-debug && cmake -GNinja -Wno-dev -DNO_WALL=ON -DCMAKE_BUILD_TYPE=Debug $(DEBUG_FLAGS) $(CMAKE_FLAGS) -DBUILD_TESTS=ON .. && ninja $(NINJA_FLAGS) $1 install
endef

define cmake_build_target_release
mkdir -p build-release
cd build-release && cmake -GNinja -Wno-dev -DNO_WALL=ON -DCMAKE_BUILD_TYPE=Release $(CMAKE_FLAGS) --target -DBUILD_TESTS=ON .. && ninja $(NINJA_FLAGS) $1 install
cd build-release && cmake -GNinja -Wno-dev -DNO_WALL=ON -DCMAKE_BUILD_TYPE=Release $(CMAKE_FLAGS) -DBUILD_TESTS=ON .. && ninja $(NINJA_FLAGS) $1 install
endef

define cmake_build_target_perf
mkdir -p build-release-debug
cd build-release-debug && cmake -GNinja -Wno-dev -DNO_WALL=ON -DCMAKE_BUILD_TYPE=RelWithDebInfo $(CMAKE_FLAGS) --target -DBUILD_TESTS=ON .. && ninja $(NINJA_FLAGS) $1 install
cd build-release-debug && cmake -GNinja -Wno-dev -DNO_WALL=ON -DCMAKE_BUILD_TYPE=RelWithDebInfo $(CMAKE_FLAGS) -DBUILD_TESTS=ON .. && ninja $(NINJA_FLAGS) $1 install
endef

all-perf:
Expand Down
8 changes: 3 additions & 5 deletions rj_common/include/rj_common/time.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,11 @@ struct RosConverter<RJ::Time, builtin_interfaces::msg::Time> {
return RosConverter<RJ::Time, rclcpp::Time>::from_ros(value);
}
};

// std::chrono::duration_cast<std::chrono::nanoseconds>(value).count()
template <>
struct RosConverter<RJ::Seconds, rclcpp::Duration> {
static rclcpp::Duration to_ros(const RJ::Seconds& value) {
return rclcpp::Duration(
std::chrono::duration_cast<std::chrono::nanoseconds>(value)
.count());
return rclcpp::Duration(std::chrono::duration_cast<std::chrono::nanoseconds>(value));
}
static RJ::Seconds from_ros(const rclcpp::Duration& value) {
const std::chrono::nanoseconds dur(value.nanoseconds());
Expand All @@ -126,4 +124,4 @@ struct RosConverter<RJ::Seconds, builtin_interfaces::msg::Duration> {

ASSOCIATE_CPP_ROS(RJ::Seconds, builtin_interfaces::msg::Duration);

} // namespace rj_convert
} // namespace rj_convert
4 changes: 3 additions & 1 deletion rj_common/testing/rj_common_convert_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,7 @@ TEST(ROSConvert, duration_lossless_convert) {
// We don't expect perfect equality for seconds, because float comparisons.
using Cvt = rj_convert::RosConverter<RJ::Seconds, rclcpp::Duration>;
EXPECT_NEAR(Cvt::from_ros(Cvt::to_ros(RJ::Seconds(1.0))).count(), 1.0, 1e-6);
EXPECT_NEAR(Cvt::to_ros(Cvt::from_ros(rclcpp::Duration(12345))).nanoseconds(), 12345, 1);
EXPECT_NEAR(
Cvt::to_ros(Cvt::from_ros(rclcpp::Duration(std::chrono::nanoseconds(12345)))).nanoseconds(),
12345, 1);
}
3 changes: 2 additions & 1 deletion rj_config/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ project(rj_config)
# ======================================================================
find_package(rclcpp REQUIRED)
find_package(rj_msgs REQUIRED)
find_package(fmt REQUIRED)

# ======================================================================
# Define Targets
Expand All @@ -25,7 +26,7 @@ add_subdirectory(src)
# ======================================================================
set(RJ_CONFIG_DEPS_INCLUDE_DIRS include)

set(RJ_CONFIG_DEPS_LIBRARIES rj_constants rj_common rj_utils)
set(RJ_CONFIG_DEPS_LIBRARIES rj_constants rj_common rj_utils fmt)

# ======================================================================
# Include and Linking
Expand Down
12 changes: 6 additions & 6 deletions rj_utils/src/logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,19 @@ void Ros2Sink<Mutex>::sink_it_(const spdlog::details::log_msg& msg) {
switch (msg.level) {
case spdlog::level::trace:
case spdlog::level::debug:
RCLCPP_DEBUG(logger, string); // NOLINT
RCLCPP_DEBUG(logger, string.c_str()); // NOLINT
break;
case spdlog::level::info:
RCLCPP_INFO(logger, string); // NOLINT
RCLCPP_INFO(logger, string.c_str()); // NOLINT
break;
case spdlog::level::warn:
RCLCPP_WARN(logger, string); // NOLINT
RCLCPP_WARN(logger, string.c_str()); // NOLINT
break;
case spdlog::level::err:
RCLCPP_ERROR(logger, string); // NOLINT
RCLCPP_ERROR(logger, string.c_str()); // NOLINT
break;
case spdlog::level::critical:
RCLCPP_FATAL(logger, string); // NOLINT
RCLCPP_FATAL(logger, string.c_str()); // NOLINT
break;
case spdlog::level::off:
break;
Expand All @@ -58,4 +58,4 @@ void set_spdlog_default_ros2(const std::string& logger_name) {
ros2_logger->set_pattern("[%s:%#] %v");
spdlog::set_default_logger(std::move(ros2_logger));
}
} // namespace rj_utils
} // namespace rj_utils
14 changes: 6 additions & 8 deletions soccer/src/soccer/strategy/agent/agent_action_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,11 @@ void AgentActionClient::send_new_goal() {
goal_msg.robot_intent = rj_convert::convert_to_ros(last_task_);

auto send_goal_options = rclcpp_action::Client<RobotMove>::SendGoalOptions();
send_goal_options.goal_response_callback =
std::bind(&AgentActionClient::goal_response_callback, this, _1);
send_goal_options.feedback_callback =
std::bind(&AgentActionClient::feedback_callback, this, _1, _2);
send_goal_options.result_callback = std::bind(&AgentActionClient::result_callback, this, _1);
send_goal_options.goal_response_callback = [this](auto arg) { goal_response_callback(arg); };
send_goal_options.feedback_callback = [this](auto arg1, auto arg2) {
feedback_callback(arg1, arg2);
};
send_goal_options.result_callback = [this](auto arg) { result_callback(arg); };
client_ptr_->async_send_goal(goal_msg, send_goal_options);
}

Expand All @@ -195,9 +195,7 @@ void AgentActionClient::send_new_goal() {
// in the initializer. This will call the class which will analyze the
// situation based on the current tick.

void AgentActionClient::goal_response_callback(
std::shared_future<GoalHandleRobotMove::SharedPtr> future) {
auto goal_handle = future.get();
void AgentActionClient::goal_response_callback(GoalHandleRobotMove::SharedPtr goal_handle) {
if (!goal_handle) {
current_position_->set_goal_canceled();
}
Expand Down
2 changes: 1 addition & 1 deletion soccer/src/soccer/strategy/agent/agent_action_client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class AgentActionClient : public rclcpp::Node {

// ROS ActionClient spec, for calls to planning ActionServer
rclcpp_action::Client<RobotMove>::SharedPtr client_ptr_;
void goal_response_callback(std::shared_future<GoalHandleRobotMove::SharedPtr> future);
void goal_response_callback(GoalHandleRobotMove::SharedPtr future);
void feedback_callback(GoalHandleRobotMove::SharedPtr,
const std::shared_ptr<const RobotMove::Feedback> feedback);

Expand Down
7 changes: 4 additions & 3 deletions soccer/src/soccer/ui/field_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,18 @@

#include <QLayout>
#include <QPainter>
#include <QPainterPath>
#include <QResizeEvent>
#include <QStyleOption>

#include <rj_geometry/point.hpp>
#include <rj_geometry/segment.hpp>
#include <rj_geometry/util.hpp>
#include <log_utils.hpp>
#include <planning/motion_constraints.hpp>
#include <rj_common/field_dimensions.hpp>
#include <rj_common/vision_dot_pattern.hpp>
#include <rj_constants/constants.hpp>
#include <rj_geometry/point.hpp>
#include <rj_geometry/segment.hpp>
#include <rj_geometry/util.hpp>

using namespace std;

Expand Down
6 changes: 4 additions & 2 deletions soccer/src/soccer/ui/robot_status_widget.hpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#pragma once

#include <rj_protos/LogFrame.pb.h>
#include <optional>
#include <string>

#include <QtWidgets>
#include <string>

#include <rj_protos/LogFrame.pb.h>

#include "ui_RobotStatusWidget.h"

Expand Down
4 changes: 2 additions & 2 deletions source.bash
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
if [[ $SHELL == *"bash"* ]]; then
echo "bash detected, sourcing bash"
source /opt/ros/foxy/setup.bash
source /opt/ros/humble/setup.bash
source install/setup.bash
fi

if [[ $SHELL == *"zsh"* ]]; then
echo "zsh detected, sourcing zsh"
source /opt/ros/foxy/setup.zsh
source /opt/ros/humble/setup.zsh
source install/setup.zsh
fi
7 changes: 5 additions & 2 deletions util/ubuntu-packages.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ cmake
ccache

# QT
qt5-default
qtbase5-dev
qtchooser
qt5-qmake
qtbase5-dev-tools
qttools5-dev-tools
libqt5opengl5-dev
libqt5svg5-dev
Expand Down Expand Up @@ -83,4 +86,4 @@ freeglut3-dev
libfmt-dev

# "everything else" (aka ros)
ros-foxy-rqt*
ros-humble-rqt*
18 changes: 9 additions & 9 deletions util/ubuntu-setup
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ if cat /etc/os-release | grep -iq '^NAME=.*Debian'; then # don't add repositorie
SYSTEM="debian"
ADD_REPOS=false
elif cat /etc/os-release | grep -iq '^NAME=.*Ubuntu'; then # We are using a version of ubuntu...
if cat /etc/os-release | grep -iq '^VERSION=.*20.04'; then # Using Ubuntu 20.04
echo "Ubuntu 20.04 Detected..."
SYSTEM="ubuntu-20.04"
if cat /etc/os-release | grep -iq '^VERSION=.*22.04'; then # Using Ubuntu 20.04
echo "Ubuntu 22.04 Detected..."
SYSTEM="ubuntu-22.04"
ADD_REPOS=false
else
echo "Sorry, we only support 20.04 due to ROS2." >&2
echo "Sorry, we only support 22.04 due to ROS2." >&2
echo "$DISTRO_STR" >&2
exit 1
fi
Expand Down Expand Up @@ -108,7 +108,7 @@ fi
if $ADD_REPOS; then
# add repo for backport of cmake3 from debian testing
# TODO remove this once ubuntu ships cmake3
sudo add-apt-repository -y ppa:george-edison55/cmake-3.x
# sudo add-apt-repository -y ppa:george-edison55/cmake-3.x

# for newer compilers
add-apt-repository -y ppa:ubuntu-toolchain-r/test
Expand All @@ -128,10 +128,10 @@ apt-get install $ARGS wget curl # Somehow we don't have wget
wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -

# Install clang-format-10 using automatic llvm installation script
if [ "$SYSTEM" = "ubuntu-20.04" ]; then
if [ "$SYSTEM" = "ubuntu-22.04" ]; then
wget https://apt.llvm.org/llvm.sh
chmod +x llvm.sh
sudo ./llvm.sh 10
sudo ./llvm.sh 16
rm llvm.sh
fi

Expand All @@ -145,8 +145,8 @@ sudo sh -c 'echo "deb http://packages.ros.org/ros2/ubuntu `lsb_release -cs` main
sudo apt-get update

# Install eloquent for 18.04, foxy for 20.04
if [ "$SYSTEM" = "ubuntu-20.04" ]; then
sudo apt-get install -y ros-foxy-ros-base
if [ "$SYSTEM" = "ubuntu-22.04" ]; then
sudo apt-get install -y ros-humble-ros-base
fi

MACH=$(uname -m)
Expand Down