Add ament_cpplint test, resolve TODOs (#162)

* Add ament_cpplint test

# Conflicts:
#	package.xml

* Fix most of cpplint errors

# Conflicts:
#	include/web_video_server/h264_streamer.hpp
#	include/web_video_server/image_streamer.hpp
#	include/web_video_server/jpeg_streamers.hpp
#	include/web_video_server/libav_streamer.hpp
#	include/web_video_server/multipart_stream.hpp
#	include/web_video_server/png_streamers.hpp
#	include/web_video_server/ros_compressed_streamer.hpp
#	include/web_video_server/vp9_streamer.hpp
#	include/web_video_server/web_video_server.hpp
#	src/web_video_server.cpp

* Add throttled logs

* Remove redundant TODOs

* Replace header guards with pragma once

include/web_video_server/h264_streamer.hpp

@@ -27,10 +27,11 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef H264_STREAMERS_H_
-#define H264_STREAMERS_H_
+#pragma once
 
-#include <image_transport/image_transport.hpp>
+#include <string>
+
+#include "image_transport/image_transport.hpp"
 #include "web_video_server/libav_streamer.hpp"
 #include "async_web_server_cpp/http_request.hpp"
 #include "async_web_server_cpp/http_connection.hpp"
@@ -62,6 +63,4 @@ class H264StreamerType : public LibavStreamerType
     rclcpp::Node::SharedPtr node);
 };
 
-}
-
-#endif
+}  // namespace web_video_server

include/web_video_server/image_streamer.hpp

@@ -28,13 +28,15 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef IMAGE_STREAMER_H_
-#define IMAGE_STREAMER_H_
+#pragma once
+
+#include <string>
 
-#include <rclcpp/rclcpp.hpp>
-#include <image_transport/image_transport.hpp>
-#include <image_transport/transport_hints.hpp>
 #include <opencv2/opencv.hpp>
+
+#include "rclcpp/rclcpp.hpp"
+#include "image_transport/image_transport.hpp"
+#include "image_transport/transport_hints.hpp"
 #include "web_video_server/utils.hpp"
 #include "async_web_server_cpp/http_server.hpp"
 #include "async_web_server_cpp/http_request.hpp"
@@ -124,6 +126,4 @@ class ImageStreamerType
   virtual std::string create_viewer(const async_web_server_cpp::HttpRequest & request) = 0;
 };
 
-}
-
-#endif
+}  // namespace web_video_server

include/web_video_server/jpeg_streamers.hpp

@@ -28,10 +28,11 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef JPEG_STREAMERS_H_
-#define JPEG_STREAMERS_H_
+#pragma once
 
-#include <image_transport/image_transport.hpp>
+#include <string>
+
+#include "image_transport/image_transport.hpp"
 #include "web_video_server/image_streamer.hpp"
 #include "async_web_server_cpp/http_request.hpp"
 #include "async_web_server_cpp/http_connection.hpp"
@@ -82,6 +83,4 @@ class JpegSnapshotStreamer : public ImageTransportImageStreamer
   int quality_;
 };
 
-}
-
-#endif
+}  // namespace web_video_server

include/web_video_server/libav_streamer.hpp

@@ -28,13 +28,7 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef LIBAV_STREAMERS_H_
-#define LIBAV_STREAMERS_H_
-
-#include <image_transport/image_transport.hpp>
-#include "web_video_server/image_streamer.hpp"
-#include "async_web_server_cpp/http_request.hpp"
-#include "async_web_server_cpp/http_connection.hpp"
+#pragma once
 
 extern "C"
 {
@@ -48,6 +42,13 @@ extern "C"
 #include <libavutil/imgutils.h>
 }
 
+#include <string>
+
+#include "image_transport/image_transport.hpp"
+#include "web_video_server/image_streamer.hpp"
+#include "async_web_server_cpp/http_request.hpp"
+#include "async_web_server_cpp/http_connection.hpp"
+
 namespace web_video_server
 {
 
@@ -110,6 +111,4 @@ class LibavStreamerType : public ImageStreamerType
   const std::string content_type_;
 };
 
-}
-
-#endif
+}  // namespace web_video_server

include/web_video_server/multipart_stream.hpp

@@ -28,13 +28,15 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef MULTIPART_STREAM_H_
-#define MULTIPART_STREAM_H_
-
-#include <rclcpp/rclcpp.hpp>
-#include <async_web_server_cpp/http_connection.hpp>
+#pragma once
 
 #include <queue>
+#include <memory>
+#include <vector>
+#include <string>
+
+#include "rclcpp/rclcpp.hpp"
+#include "async_web_server_cpp/http_connection.hpp"
 
 namespace web_video_server
 {
@@ -75,6 +77,4 @@ class MultipartStream
   std::queue<PendingFooter> pending_footers_;
 };
 
-}
-
-#endif
+}  // namespace web_video_server

include/web_video_server/png_streamers.hpp

@@ -27,10 +27,11 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef PNG_STREAMERS_H_
-#define PNG_STREAMERS_H_
+#pragma once
 
-#include <image_transport/image_transport.hpp>
+#include <string>
+
+#include "image_transport/image_transport.hpp"
 #include "web_video_server/image_streamer.hpp"
 #include "async_web_server_cpp/http_request.hpp"
 #include "async_web_server_cpp/http_connection.hpp"
@@ -83,6 +84,4 @@ class PngSnapshotStreamer : public ImageTransportImageStreamer
   int quality_;
 };
 
-}
-
-#endif
+}  // namespace web_video_server

include/web_video_server/ros_compressed_streamer.hpp

@@ -28,10 +28,11 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef ROS_COMPRESSED_STREAMERS_H_
-#define ROS_COMPRESSED_STREAMERS_H_
+#pragma once
 
-#include <sensor_msgs/msg/compressed_image.hpp>
+#include <string>
+
+#include "sensor_msgs/msg/compressed_image.hpp"
 #include "web_video_server/image_streamer.hpp"
 #include "async_web_server_cpp/http_request.hpp"
 #include "async_web_server_cpp/http_connection.hpp"
@@ -76,6 +77,4 @@ class RosCompressedStreamerType : public ImageStreamerType
   std::string create_viewer(const async_web_server_cpp::HttpRequest & request);
 };
 
-}
-
-#endif
+}  // namespace web_video_server

include/web_video_server/vp8_streamer.hpp

@@ -28,10 +28,11 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef VP8_STREAMERS_H_
-#define VP8_STREAMERS_H_
+#pragma once
 
-#include <image_transport/image_transport.hpp>
+#include <string>
+
+#include "image_transport/image_transport.hpp"
 #include "web_video_server/libav_streamer.hpp"
 #include "async_web_server_cpp/http_request.hpp"
 #include "async_web_server_cpp/http_connection.hpp"
@@ -65,6 +66,4 @@ class Vp8StreamerType : public LibavStreamerType
     rclcpp::Node::SharedPtr node);
 };
 
-}
-
-#endif
+}  // namespace web_video_server

include/web_video_server/vp9_streamer.hpp

@@ -27,10 +27,9 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef VP9_STREAMERS_H_
-#define VP9_STREAMERS_H_
+#pragma once
 
-#include <image_transport/image_transport.hpp>
+#include "image_transport/image_transport.hpp"
 #include "web_video_server/libav_streamer.hpp"
 #include "async_web_server_cpp/http_request.hpp"
 #include "async_web_server_cpp/http_connection.hpp"
@@ -61,6 +60,4 @@ class Vp9StreamerType : public LibavStreamerType
     rclcpp::Node::SharedPtr node);
 };
 
-}
-
-#endif
+}  // namespace web_video_server

include/web_video_server/web_video_server.hpp

@@ -28,18 +28,19 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef WEB_VIDEO_SERVER_H_
-#define WEB_VIDEO_SERVER_H_
+#pragma once
 
-#include <rclcpp/rclcpp.hpp>
+#include <map>
+#include <string>
+#include <vector>
 
 #ifdef CV_BRIDGE_USES_OLD_HEADERS
-#include <cv_bridge/cv_bridge.h>
+#include "cv_bridge/cv_bridge.h"
 #else
-#include <cv_bridge/cv_bridge.hpp>
+#include "cv_bridge/cv_bridge.hpp"
 #endif
 
-#include <vector>
+#include "rclcpp/rclcpp.hpp"
 #include "web_video_server/image_streamer.hpp"
 #include "async_web_server_cpp/http_server.hpp"
 #include "async_web_server_cpp/http_request.hpp"
@@ -59,7 +60,7 @@ class WebVideoServer
    * @brief  Constructor
    * @return
    */
-  WebVideoServer(rclcpp::Node::SharedPtr & node);
+  explicit WebVideoServer(rclcpp::Node::SharedPtr & node);
 
   /**
    * @brief  Destructor - Cleans up
@@ -121,6 +122,4 @@ class WebVideoServer
   boost::mutex subscriber_mutex_;
 };
 
-}
-
-#endif
+}  // namespace web_video_server

package.xml

@@ -33,6 +33,7 @@
 
   <test_depend>ament_lint_auto</test_depend>
   <test_depend>ament_cmake_copyright</test_depend>
+  <test_depend>ament_cmake_cpplint</test_depend>
   <test_depend>ament_cmake_lint_cmake</test_depend>
   <test_depend>ament_cmake_xmllint</test_depend>
   <test_depend>ament_cmake_uncrustify</test_depend>

src/h264_streamer.cpp

@@ -77,4 +77,4 @@ boost::shared_ptr<ImageStreamer> H264StreamerType::create_streamer(
   return boost::shared_ptr<ImageStreamer>(new H264Streamer(request, connection, node));
 }
 
-}
+}  // namespace web_video_server

src/image_streamer.cpp

@@ -118,21 +118,22 @@ void ImageTransportImageStreamer::restreamFrame(double max_age)
   try {
     if (last_frame + rclcpp::Duration::from_seconds(max_age) < node_->now() ) {
       boost::mutex::scoped_lock lock(send_mutex_);
-      sendImage(output_size_image, node_->now() ); // don't update last_frame, it may remain an old value.
+      // don't update last_frame, it may remain an old value.
+      sendImage(output_size_image, node_->now());
     }
   } catch (boost::system::system_error & e) {
     // happens when client disconnects
     RCLCPP_DEBUG(node_->get_logger(), "system_error exception: %s", e.what());
     inactive_ = true;
     return;
   } catch (std::exception & e) {
-    // TODO THROTTLE with 30
-    RCLCPP_ERROR(node_->get_logger(), "exception: %s", e.what());
+    auto & clk = *node_->get_clock();
+    RCLCPP_ERROR_THROTTLE(node_->get_logger(), clk, 40, "exception: %s", e.what());
     inactive_ = true;
     return;
   } catch (...) {
-    // TODO THROTTLE with 30
-    RCLCPP_ERROR(node_->get_logger(), "exception");
+    auto & clk = *node_->get_clock();
+    RCLCPP_ERROR_THROTTLE(node_->get_logger(), clk, 40, "exception");
     inactive_ = true;
     return;
   }
@@ -183,7 +184,7 @@ void ImageTransportImageStreamer::imageCallback(const sensor_msgs::msg::Image::C
       cv::flip(img, img, true);
     }
 
-    boost::mutex::scoped_lock lock(send_mutex_); // protects output_size_image
+    boost::mutex::scoped_lock lock(send_mutex_);  // protects output_size_image
     if (output_width_ != input_width || output_height_ != input_height) {
       cv::Mat img_resized;
       cv::Size new_size(output_width_, output_height_);
@@ -201,13 +202,13 @@ void ImageTransportImageStreamer::imageCallback(const sensor_msgs::msg::Image::C
     last_frame = node_->now();
     sendImage(output_size_image, msg->header.stamp);
   } catch (cv_bridge::Exception & e) {
-    // TODO THROTTLE with 30
-    RCLCPP_ERROR(node_->get_logger(), "cv_bridge exception: %s", e.what());
+    auto & clk = *node_->get_clock();
+    RCLCPP_ERROR_THROTTLE(node_->get_logger(), clk, 40, "cv_bridge exception: %s", e.what());
     inactive_ = true;
     return;
   } catch (cv::Exception & e) {
-    // TODO THROTTLE with 30
-    RCLCPP_ERROR(node_->get_logger(), "cv_bridge exception: %s", e.what());
+    auto & clk = *node_->get_clock();
+    RCLCPP_ERROR_THROTTLE(node_->get_logger(), clk, 40, "cv_bridge exception: %s", e.what());
     inactive_ = true;
     return;
   } catch (boost::system::system_error & e) {
@@ -216,16 +217,16 @@ void ImageTransportImageStreamer::imageCallback(const sensor_msgs::msg::Image::C
     inactive_ = true;
     return;
   } catch (std::exception & e) {
-    // TODO THROTTLE with 30
-    RCLCPP_ERROR(node_->get_logger(), "exception: %s", e.what());
+    auto & clk = *node_->get_clock();
+    RCLCPP_ERROR_THROTTLE(node_->get_logger(), clk, 40, "exception: %s", e.what());
     inactive_ = true;
     return;
   } catch (...) {
-    // TODO THROTTLE with 30
-    RCLCPP_ERROR(node_->get_logger(), "exception");
+    auto & clk = *node_->get_clock();
+    RCLCPP_ERROR_THROTTLE(node_->get_logger(), clk, 40, "exception");
     inactive_ = true;
     return;
   }
 }
 
-}
+}  // namespace web_video_server

src/jpeg_streamers.cpp

@@ -47,7 +47,7 @@ MjpegStreamer::MjpegStreamer(
 MjpegStreamer::~MjpegStreamer()
 {
   this->inactive_ = true;
-  boost::mutex::scoped_lock lock(send_mutex_); // protects sendImage.
+  boost::mutex::scoped_lock lock(send_mutex_);  // protects sendImage.
 }
 
 void MjpegStreamer::sendImage(const cv::Mat & img, const rclcpp::Time & time)
@@ -91,7 +91,7 @@ JpegSnapshotStreamer::JpegSnapshotStreamer(
 JpegSnapshotStreamer::~JpegSnapshotStreamer()
 {
   this->inactive_ = true;
-  boost::mutex::scoped_lock lock(send_mutex_); // protects sendImage.
+  boost::mutex::scoped_lock lock(send_mutex_);  // protects sendImage.
 }
 
 void JpegSnapshotStreamer::sendImage(const cv::Mat & img, const rclcpp::Time & time)
@@ -104,7 +104,7 @@ void JpegSnapshotStreamer::sendImage(const cv::Mat & img, const rclcpp::Time & t
   cv::imencode(".jpeg", img, encoded_buffer, encode_params);
 
   char stamp[20];
-  sprintf(stamp, "%.06lf", time.seconds());
+  snprintf(stamp, sizeof(stamp), "%.06lf", time.seconds());
   async_web_server_cpp::HttpReply::builder(async_web_server_cpp::HttpReply::ok)
   .header("Connection", "close")
   .header("Server", "web_video_server")
@@ -121,4 +121,4 @@ void JpegSnapshotStreamer::sendImage(const cv::Mat & img, const rclcpp::Time & t
   inactive_ = true;
 }
 
-}
+}  // namespace web_video_server