engine: Fix some findings from clang-tidy

Author: cassava

engine/src/config.hpp

@@ -49,8 +49,20 @@
 #define CLOE_SIMULATION_UUID_VAR "CLOE_SIMULATION_UUID"
 #endif
 
+#ifndef CLOE_LUA_DEBUGGER_PORT
+#define CLOE_LUA_DEBUGGER_PORT 21110
+#endif
+
 // The environment variable from which additional plugins should
 // be loaded. Takes the same format as PATH.
 #ifndef CLOE_PLUGIN_PATH
 #define CLOE_PLUGIN_PATH "CLOE_PLUGIN_PATH"
 #endif
+
+#ifndef CLOE_TRIGGER_PATH_DELIMITER
+#define CLOE_TRIGGER_PATH_DELIMITER "/"
+#endif
+
+#ifndef CLOE_SIGNAL_PATH_DELIMITER
+#define CLOE_SIGNAL_PATH_DELIMITER "/"
+#endif

engine/src/coordinator.hpp

@@ -49,12 +49,12 @@ class TriggerUnknownAction : public cloe::TriggerInvalid {
  public:
   TriggerUnknownAction(const std::string& key, const cloe::Conf& c)
       : TriggerInvalid(c, "unknown action: " + key), key_(key) {}
-  virtual ~TriggerUnknownAction() noexcept = default;
+   ~TriggerUnknownAction() noexcept override = default;
 
   /**
    * Return key that is unknown.
    */
-  const char* key() const { return key_.c_str(); }
+  [[nodiscard]] const char* key() const { return key_.c_str(); }
 
  private:
   std::string key_;
@@ -73,7 +73,7 @@ class TriggerUnknownEvent : public cloe::TriggerInvalid {
   /**
    * Return key that is unknown.
    */
-  const char* key() const { return key_.c_str(); }
+  [[nodiscard]] const char* key() const { return key_.c_str(); }
 
  private:
   std::string key_;
@@ -107,15 +107,15 @@ class Coordinator {
   void register_event(const std::string& key, cloe::EventFactoryPtr&& ef,
                       std::shared_ptr<cloe::Callback> storage);
 
-  sol::table register_lua_table(const std::string& field);
+  [[nodiscard]] sol::table register_lua_table(const std::string& field);
 
-  cloe::DataBroker* data_broker() const { return db_; }
+  [[nodiscard]] cloe::DataBroker* data_broker() const { return db_; }
 
   std::shared_ptr<cloe::TriggerRegistrar> trigger_registrar(cloe::Source s);
 
   void enroll(cloe::Registrar& r);
 
-  cloe::Logger logger() const { return cloe::logger::get("cloe"); }
+  [[nodiscard]] cloe::Logger logger() const { return cloe::logger::get("cloe"); }
 
   /**
    * Process any incoming triggers, clear the buffer, and trigger time-based
@@ -130,11 +130,11 @@ class Coordinator {
   void execute_action_from_lua(const cloe::Sync& sync, const sol::object& obj);
 
  protected:
-  cloe::ActionPtr make_action(const sol::object& lua) const;
-  cloe::ActionPtr make_action(const cloe::Conf& c) const;
-  cloe::EventPtr make_event(const cloe::Conf& c) const;
-  cloe::TriggerPtr make_trigger(cloe::Source s, const cloe::Conf& c) const;
-  cloe::TriggerPtr make_trigger(const sol::table& tbl) const;
+  [[nodiscard]] cloe::ActionPtr make_action(const sol::object& lua) const;
+  [[nodiscard]] cloe::ActionPtr make_action(const cloe::Conf& c) const;
+  [[nodiscard]] cloe::EventPtr make_event(const cloe::Conf& c) const;
+  [[nodiscard]] cloe::TriggerPtr make_trigger(cloe::Source s, const cloe::Conf& c) const;
+  [[nodiscard]] cloe::TriggerPtr make_trigger(const sol::table& tbl) const;
   void queue_trigger(cloe::Source s, const cloe::Conf& c) { queue_trigger(make_trigger(s, c)); }
   void queue_trigger(cloe::TriggerPtr&& tp);
   void store_trigger(cloe::TriggerPtr&& tp, const cloe::Sync& sync);

engine/src/error_handler.hpp

@@ -18,14 +18,21 @@
 
 #pragma once
 
-#include <iostream>
+#include <ostream>  // for ostream
+#include <sstream>  // for stringstream
 
 #include <cloe/core/error.hpp>  // for Error
 #include <fable/error.hpp>      // for ConfError, SchemaError
 #include <fable/utility.hpp>    // for indent_string, pretty_print
 
 namespace cloe {
 
+/**
+ * Format various kinds of error so that they are easy to read.
+ *
+ * \param exception error to format
+ * \return formatted string, ready for printing
+ */
 inline std::string format_error(const std::exception& exception) {
   std::stringstream buf;
   if (const auto* err = dynamic_cast<const fable::SchemaError*>(&exception); err) {
@@ -44,6 +51,21 @@ inline std::string format_error(const std::exception& exception) {
   return buf.str();
 }
 
+/**
+ * Run a function and print any exception nicely to the ostream provided.
+ *
+ * This essentially replaces:
+ *
+ *     try { ... }
+ *     catch (cloe::ConcludedError&) { ... }
+ *     catch (std::exception&) { ... }
+ *
+ * with a single line.
+ *
+ * \param out stream to write error message to (e.g. std::cerr)
+ * \param f function to run
+ * \return return value of f
+ */
 template <typename Func>
 auto conclude_error(std::ostream& out, Func f) -> decltype(f()) {
   try {

engine/src/lua_stack_test.cpp

@@ -1,3 +1,20 @@
+/*
+ * Copyright 2024 Robert Bosch GmbH
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
 
 #include <gtest/gtest.h>
 #include <string>

engine/src/main_commands.hpp

@@ -24,6 +24,7 @@
 #include <string>
 #include <vector>
 
+#include "config.hpp"
 #include "lua_setup.hpp"
 #include "stack_factory.hpp"
 
@@ -77,7 +78,7 @@ struct RunOptions {
   bool report_progress = true;
 
   bool debug_lua = false;
-  int debug_lua_port = 21110;
+  int debug_lua_port = CLOE_LUA_DEBUGGER_PORT;
 };
 
 int run(const RunOptions& opt, const std::vector<std::string>& filepaths);

engine/src/server.cpp

@@ -36,9 +36,8 @@ namespace engine {
 
 class ServerRegistrarImpl : public ServerRegistrar {
  public:
-  ServerRegistrarImpl(
-      oak::Registrar static_reg, oak::ProxyRegistrar<cloe::HandlerType> api_reg)
-      : static_registrar_(static_reg), api_registrar_(api_reg) {}
+  ServerRegistrarImpl(const oak::Registrar& static_reg, oak::ProxyRegistrar<cloe::HandlerType> api_reg)
+      : static_registrar_(static_reg), api_registrar_(std::move(api_reg)) {}
 
   std::unique_ptr<ServerRegistrar> clone() const override {
     return std::make_unique<ServerRegistrarImpl>(static_registrar_, api_registrar_);
@@ -62,7 +61,7 @@ class ServerRegistrarImpl : public ServerRegistrar {
   }
 
   void register_api_handler(const std::string& endpoint, cloe::HandlerType t,
-                                    cloe::Handler h) override {
+                            cloe::Handler h) override {
     api_registrar_.register_handler(endpoint, t, h);
   }
 

engine/src/server.hpp

@@ -41,9 +41,9 @@ class ServerRegistrar {
  public:
   virtual ~ServerRegistrar() = default;
 
-  virtual std::unique_ptr<ServerRegistrar> clone() const = 0;
+  [[nodiscard]] virtual std::unique_ptr<ServerRegistrar> clone() const = 0;
 
-  virtual std::unique_ptr<ServerRegistrar> with_prefix(const std::string& static_prefix,
+  [[nodiscard]] virtual std::unique_ptr<ServerRegistrar> with_prefix(const std::string& static_prefix,
                                                        const std::string& api_prefix) const = 0;
 
   virtual void register_static_handler(const std::string& endpoint, cloe::Handler h) = 0;
@@ -59,25 +59,29 @@ class ServerRegistrar {
  */
 class Server {
  public:
-  Server(const cloe::ServerConf& config) : config_(config) {}
+  Server(const Server&) = default;
+  Server(Server&&) = delete;
+  Server& operator=(const Server&) = default;
+  Server& operator=(Server&&) = delete;
+  Server(cloe::ServerConf config) : config_(std::move(config)) {}
   virtual ~Server() = default;
 
   /**
    * Return the server configuration.
    */
-  const cloe::ServerConf& config() const { return config_; }
+  [[nodiscard]] const cloe::ServerConf& config() const { return config_; }
 
   /**
    * Return whether the server is alive and listening for requests.
    */
-  virtual bool is_listening() const = 0;
+  [[nodiscard]] virtual bool is_listening() const = 0;
 
   /**
    * Return whether the server is currently streaming buffer data to a file.
    *
    * If it is, expect performance to be bad.
    */
-  virtual bool is_streaming() const = 0;
+  [[nodiscard]] virtual bool is_streaming() const = 0;
 
   /**
    * Start the web server.
@@ -104,7 +108,7 @@ class Server {
    * Return a new ServerRegistrar that lets you register static content and
    * API endpoints with the web server.
    */
-  virtual std::unique_ptr<ServerRegistrar> server_registrar() = 0;
+  [[nodiscard]] virtual std::unique_ptr<ServerRegistrar> server_registrar() = 0;
 
   /**
    * Refresh and/or start streaming api data to a file.
@@ -124,7 +128,7 @@ class Server {
    *
    * \return Lock guard
    */
-  virtual Defer lock() = 0;
+  [[nodiscard]] virtual Defer lock() = 0;
 
  protected:
   cloe::Logger logger() const { return cloe::logger::get("cloe"); }

oak/include/oak/server.hpp

@@ -71,19 +71,19 @@ class Server {
    */
   void set_address(const std::string& addr) { listen_addr_ = addr; }
 
-  const std::string& address() const { return listen_addr_; }
+  [[nodiscard]] const std::string& address() const { return listen_addr_; }
 
   /**
    * Set the port on which to listen.
    */
   void set_port(int port) { listen_port_ = port; }
 
-  int port() const { return listen_port_; }
+  [[nodiscard]] int port() const { return listen_port_; }
 
   /**
    * Returns whether the server has started and is currently listening.
    */
-  bool is_listening() const { return listening_; }
+  [[nodiscard]] bool is_listening() const { return listening_; }
 
   /**
    * Start the server.
@@ -93,7 +93,7 @@ class Server {
   /**
    * Return endpoint data in json format.
    */
-  fable::Json endpoints_to_json(const std::vector<std::string>& endpoints) const;
+  [[nodiscard]] fable::Json endpoints_to_json(const std::vector<std::string>& endpoints) const;
 
   /**
    * Stop the server.
@@ -103,7 +103,7 @@ class Server {
   /**
    * Return a list of all registered endpoints.
    */
-  std::vector<std::string> endpoints() const;
+  [[nodiscard]] std::vector<std::string> endpoints() const;
 
  protected:
   friend StaticRegistrar;