From 7ab331d9c5aa00d0bbb5eb036644e454626382e9 Mon Sep 17 00:00:00 2001 From: Khalil Estell Date: Fri, 4 Aug 2023 07:27:42 -0700 Subject: [PATCH] :fire: Remove can_router from util can_router is misplaced in this library. Utilities, at their core, are optional constructs. They provide helper functions for developers. can_router is an interface requirement for drivers which does not fit the design of util. Thus it will be decoupled from libhal-util and brought into libhal-canrouter. Add unit tests for can message comparison --- .github/workflows/ci.yml | 15 +- conanfile.py | 2 +- include/libhal-util/can.hpp | 150 -------------- tests/CMakeLists.txt | 11 +- tests/can.test.cpp | 393 ++---------------------------------- tests/main.test.cpp | 4 +- 6 files changed, 32 insertions(+), 543 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 81f364e..5536cb8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,9 +3,9 @@ name: ✅ Checks on: workflow_dispatch: pull_request: + release: + types: [published] push: - tags: - - "*" branches: - main schedule: @@ -19,14 +19,3 @@ jobs: devices: uses: libhal/ci/.github/workflows/deploy.yml@4.0.0 secrets: inherit - - release: - needs: [ci, devices] - if: startsWith(github.ref, 'refs/tags/') - runs-on: ubuntu-22.04 - steps: - - name: Release - uses: softprops/action-gh-release@v1 - if: startsWith(github.ref, 'refs/tags/') - with: - generate_release_notes: true diff --git a/conanfile.py b/conanfile.py index 7b1e2cc..7fd11d3 100644 --- a/conanfile.py +++ b/conanfile.py @@ -26,7 +26,7 @@ class libhal_util_conan(ConanFile): name = "libhal-util" - version = "2.0.0" + version = "3.0.0" license = "Apache-2.0" url = "https://github.com/conan-io/conan-center-index" homepage = "https://github.com/libhal/libhal-util" diff --git a/include/libhal-util/can.hpp b/include/libhal-util/can.hpp index 7628f8a..e86aa3d 100644 --- a/include/libhal-util/can.hpp +++ b/include/libhal-util/can.hpp @@ -15,15 +15,11 @@ #pragma once #include -#include -#include #include #include "comparison.hpp" #include "math.hpp" -#include "static_callable.hpp" -#include "static_list.hpp" namespace hal { [[nodiscard]] constexpr auto operator==(const can::settings& p_lhs, @@ -119,150 +115,4 @@ namespace hal { p_lhs.length == p_rhs.length && p_lhs.is_remote_request == p_rhs.is_remote_request; } - -/** - * @brief Route CAN messages received on the can bus to callbacks based on ID. - * - */ -class can_router -{ -public: - static constexpr auto noop = - []([[maybe_unused]] const can::message_t& p_message) {}; - - using message_handler = hal::callback; - - struct route - { - hal::can::id_t id = 0; - message_handler handler = noop; - }; - - using route_item = static_list::item; - - static result create(hal::can& p_can) - { - can_router new_can_router(p_can); - return new_can_router; - } - - /** - * @brief Construct a new can message router - * - * @param p_can - can peripheral to route messages for - */ - explicit can_router(hal::can& p_can) - : m_can(&p_can) - { - (void)m_can->on_receive(std::ref((*this))); - } - - can_router() = delete; - can_router(can_router& p_other) = delete; - can_router& operator=(can_router& p_other) = delete; - can_router& operator=(can_router&& p_other) - { - m_handlers = std::move(p_other.m_handlers); - m_can = std::move(p_other.m_can); - (void)m_can->on_receive(std::ref(*this)); - - p_other.m_can = nullptr; - return *this; - } - - can_router(can_router&& p_other) - { - *this = std::move(p_other); - } - - ~can_router() - { - if (m_can) { - // Assume that if this succeeded in the create factory function, that it - // will work this time - (void)m_can->on_receive(noop); - } - } - - /** - * @brief Get a reference to the can peripheral driver - * - * Used to send can messages through the same port that the can_router is - * using. - * - * @return can& reference to the can peripheral driver - */ - [[nodiscard]] hal::can& bus() - { - return *m_can; - } - - /** - * @brief Add a message route without setting the callback - * - * The default callback will do nothing and will drop the message. - * - * @param p_id - Associated ID of messages to be stored. - * @return auto - route item from the linked list that must be stored stored - * in a variable - */ - [[nodiscard]] auto add_message_callback(hal::can::id_t p_id) - { - return m_handlers.push_back(route{ - .id = p_id, - }); - } - - /** - * @brief Set a callback for when messages with a specific ID is received - * - * @param p_id - Associated ID of messages to be stored. - * @param p_handler - callback to be executed when a p_id message is received. - * @return auto - route item from the linked list that must be stored stored - * in a variable - */ - [[nodiscard]] auto add_message_callback(hal::can::id_t p_id, - message_handler p_handler) - { - return m_handlers.push_back(route{ - .id = p_id, - .handler = p_handler, - }); - } - - /** - * @brief Get the list of handlers - * - * Meant for testing purposes or when direct inspection of the map is useful - * in userspace. Should not be used in by libraries. - * - * @return const auto& map of all of the can message handlers. - */ - [[nodiscard]] const auto& handlers() - { - return m_handlers; - } - - /** - * @brief Message routing interrupt service handler - * - * Searches the static list and finds the first ID associated with the message - * and run's that route's callback. - * - * @param p_message - message received from the bus - */ - void operator()(const can::message_t& p_message) - { - for (auto& list_handler : m_handlers) { - if (p_message.id == list_handler.id) { - list_handler.handler(p_message); - return; - } - } - } - -private: - static_list m_handlers{}; - hal::can* m_can = nullptr; -}; } // namespace hal \ No newline at end of file diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e4eb064..22623f0 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -43,9 +43,15 @@ find_package(ut REQUIRED CONFIG) find_package(libhal REQUIRED CONFIG) add_executable(${PROJECT_NAME} + + # Source + ../src/steady_clock.cpp + ../src/streams.cpp + + # Tests as_bytes.test.cpp - bit.test.cpp can.test.cpp + bit.test.cpp enum.test.cpp i2c.test.cpp input_pin.test.cpp @@ -86,5 +92,4 @@ target_link_options(${PROJECT_NAME} PRIVATE ${ASAN_FLAG}) target_link_libraries(${PROJECT_NAME} PRIVATE boost-ext-ut::ut - libhal::libhal - libhal-util) + libhal::libhal) diff --git a/tests/can.test.cpp b/tests/can.test.cpp index e04f4fc..d7ddce8 100644 --- a/tests/can.test.cpp +++ b/tests/can.test.cpp @@ -1,392 +1,37 @@ -// Copyright 2023 Google LLC -// -// 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. - #include -#include - -#include - #include namespace hal { -namespace { -class mock_can : public hal::can -{ -public: - settings m_settings{}; - message_t m_message{}; - std::function m_handler{}; - bool m_return_error_status{ false }; - size_t m_on_receive_call_count = 0; - bool m_noop_set = false; - -private: - status driver_configure(const settings& p_settings) override - { - m_settings = p_settings; - if (m_return_error_status) { - return hal::new_error(); - } - return success(); - }; - - status driver_bus_on() override - { - return success(); - } - - result driver_send(const message_t& p_message) override - { - m_message = p_message; - if (m_return_error_status) { - return hal::new_error(); - } - return send_t{}; - }; - - void driver_on_receive(hal::callback p_handler) override - { - m_on_receive_call_count++; - m_handler = p_handler; - }; -}; -} // namespace - -void can_router_test() +void can_test() { using namespace boost::ut; - "operator==(can::settings)"_test = []() { - can::settings a{}; - can::settings b{}; - - expect(a == b); - }; - - "operator!=(can::settings)"_test = []() { - can::settings a{ .baud_rate = 100.0_kHz }; - can::settings b{ .baud_rate = 1200.0_kHz }; - - expect(a != b); - }; - - "can_router::create + update_callback()"_test = []() { - // Setup - mock_can mock; - - // Exercise - auto router = can_router::create(mock); - - // Verify - expect(bool{ router }); - }; - - "can_router::bus()"_test = []() { - // Setup - static constexpr can::message_t expected{ + "operator==(can::message, can::message) "_test = []() { + can::message_t a = { .id = 0x111, - .payload = { 0xAA, 0xBB, 0xCC }, - .length = 3, + .payload = { 0xAA }, + .length = 1, + .is_remote_request = false, }; - mock_can mock; - auto router = can_router::create(mock).value(); - - // Exercise - auto result = router.bus().send(expected); - - // Verify - expect(bool{ result }); - expect(expected == mock.m_message); - }; - "can_router::bus() failed"_test = []() { - // Setup - static constexpr can::message_t expected{ + can::message_t b = { .id = 0x111, - .payload = { 0xAA, 0xBB, 0xCC }, - .length = 3, + .payload = { 0xAA }, + .length = 1, + .is_remote_request = false, }; - mock_can mock; - auto router = can_router::create(mock).value(); - mock.m_return_error_status = true; - - // Exercise - auto result = router.bus().send(expected); - - // Verify - expect(!bool{ result }); - expect(expected == mock.m_message); - }; - - "can_router::add_message_callback(id)"_test = []() { - // Setup - static constexpr can::id_t id = 0x15; - mock_can mock; - auto router = can_router::create(mock).value(); - - // Exercise - auto callback_item = router.add_message_callback(id); - // Verify - expect(that % id == callback_item.get().id); - expect(that % 1 == router.handlers().size()); - - [[maybe_unused]] const auto& iterator = std::find_if( - router.handlers().begin(), - router.handlers().cend(), - [](const can_router::route& p_route) { return p_route.id == id; }); - expect(iterator->id == callback_item.get().id); - }; - - "can_router::add_message_callback(id, callback)"_test = []() { - // Setup - static constexpr can::id_t id = 0x15; - static constexpr can::message_t expected{ - .id = 0x111, - .payload = { 0xAA, 0xBB, 0xCC }, - .length = 3, + can::message_t c = { + .id = 0x112, + .payload = { 0xAB }, + .length = 1, + .is_remote_request = false, }; - mock_can mock; - auto router = can_router::create(mock).value(); - int counter = 0; - can::message_t actual{}; - - // Exercise - auto callback_item = router.add_message_callback( - id, - [&counter, &actual]([[maybe_unused]] const can::message_t& p_message) { - counter++; - actual = p_message; - }); - [[maybe_unused]] const auto& iterator = std::find_if( - router.handlers().begin(), - router.handlers().cend(), - [](const can_router::route& p_route) { return p_route.id == id; }); - - iterator->handler(expected); - - // Verify - expect(that % id == callback_item.get().id); - expect(that % 1 == router.handlers().size()); - expect(iterator->id == callback_item.get().id); - expect(that % 1 == counter); - expect(expected == actual); - }; - - "can_router::add_message_callback(id, callback)"_test = []() { - // Setup - mock_can mock; - auto router = can_router::create(mock).value(); - int counter1 = 0; - int counter2 = 0; - int counter3 = 0; - static constexpr can::message_t expected1{ - .id = 0x100, - .payload = { 0xAA, 0xBB }, - .length = 2, - }; - static constexpr can::message_t expected2{ - .id = 0x120, - .payload = { 0xCC, 0xDD }, - .length = 2, - }; - static constexpr can::message_t expected3{ - .id = 0x123, - .payload = { 0xEE, 0xFF }, - .length = 2, - }; - - can::message_t actual1{}; - can::message_t actual2{}; - can::message_t actual3{}; - - auto message_handler1 = router.add_message_callback( - expected1.id, [&counter1, &actual1](const can::message_t& p_message) { - counter1++; - actual1 = p_message; - }); - - auto message_handler2 = router.add_message_callback( - expected2.id, [&counter2, &actual2](const can::message_t& p_message) { - counter2++; - actual2 = p_message; - }); - - auto message_handler3 = router.add_message_callback( - expected3.id, [&counter3, &actual3](const can::message_t& p_message) { - counter3++; - actual3 = p_message; - }); - - // Exercise - router(expected1); - - // Verify - expect(that % 3 == router.handlers().size()); - expect(that % 1 == counter1); - expect(expected1 == actual1); - expect(that % 0 == counter2); - expect(expected2 != actual2); - expect(that % 0 == counter2); - expect(expected3 != actual3); - - router(expected2); - - expect(that % 1 == counter1); - expect(expected1 == actual1); - expect(that % 1 == counter2); - expect(expected2 == actual2); - expect(that % 0 == counter3); - expect(expected3 != actual3); - - router(expected3); - - expect(that % 1 == counter1); - expect(expected1 == actual1); - expect(that % 1 == counter2); - expect(expected2 == actual2); - expect(that % 1 == counter2); - expect(expected3 == actual3); - - router(expected2); - - expect(that % 1 == counter1); - expect(expected1 == actual1); - expect(that % 2 == counter2); - expect(expected2 == actual2); - expect(that % 1 == counter3); - expect(expected3 == actual3); - }; - - "can_router::~can_router()"_test = []() { - // Setup - mock_can mock; - { - auto router = can_router::create(mock); - // m_on_receive_call_count = 1 on construction - // m_on_receive_call_count = 2 on move - expect(that % 2 == mock.m_on_receive_call_count); - // Exercise: end of scope calls destructor - } - - // Verify - expect(that % 3 == mock.m_on_receive_call_count); - }; - - "can_router::can_router(&&) move constructor"_test = []() { - // Setup - mock_can mock; - - { - can_router original_router(mock); - expect(that % 1 == mock.m_on_receive_call_count); - - auto new_router = std::move(original_router); - expect(that % 2 == mock.m_on_receive_call_count); - } - - // Exercise: end of scope calls destructor - // Verify - expect(that % 3 == mock.m_on_receive_call_count); - }; - - "can_router::add_message_callback(id, callback) with move"_test = []() { - // Setup - mock_can mock; - auto to_be_moved_router = can_router::create(mock).value(); - auto router = std::move(to_be_moved_router); - int counter1 = 0; - int counter2 = 0; - int counter3 = 0; - static constexpr can::message_t expected1{ - .id = 0x100, - .payload = { 0xAA, 0xBB }, - .length = 2, - }; - static constexpr can::message_t expected2{ - .id = 0x120, - .payload = { 0xCC, 0xDD }, - .length = 2, - }; - static constexpr can::message_t expected3{ - .id = 0x123, - .payload = { 0xEE, 0xFF }, - .length = 2, - }; - - can::message_t actual1{}; - can::message_t actual2{}; - can::message_t actual3{}; - - auto message_handler1 = router.add_message_callback( - expected1.id, [&counter1, &actual1](const can::message_t& p_message) { - counter1++; - actual1 = p_message; - }); - - auto message_handler2 = router.add_message_callback( - expected2.id, [&counter2, &actual2](const can::message_t& p_message) { - counter2++; - actual2 = p_message; - }); - - auto message_handler3 = router.add_message_callback( - expected3.id, [&counter3, &actual3](const can::message_t& p_message) { - counter3++; - actual3 = p_message; - }); - - // Exercise - router(expected1); - - // Verify - expect(that % 3 == router.handlers().size()); - expect(that % 1 == counter1); - expect(expected1 == actual1); - expect(that % 0 == counter2); - expect(expected2 != actual2); - expect(that % 0 == counter2); - expect(expected3 != actual3); - - router(expected2); - - expect(that % 1 == counter1); - expect(expected1 == actual1); - expect(that % 1 == counter2); - expect(expected2 == actual2); - expect(that % 0 == counter3); - expect(expected3 != actual3); - - router(expected3); - - expect(that % 1 == counter1); - expect(expected1 == actual1); - expect(that % 1 == counter2); - expect(expected2 == actual2); - expect(that % 1 == counter2); - expect(expected3 == actual3); - - router(expected2); - - expect(that % 1 == counter1); - expect(expected1 == actual1); - expect(that % 2 == counter2); - expect(expected2 == actual2); - expect(that % 1 == counter3); - expect(expected3 == actual3); + expect(a == b); + expect(a != c); + expect(b != c); }; -}; +} } // namespace hal \ No newline at end of file diff --git a/tests/main.test.cpp b/tests/main.test.cpp index 840876e..9336a29 100644 --- a/tests/main.test.cpp +++ b/tests/main.test.cpp @@ -15,7 +15,7 @@ namespace hal { extern void as_bytes_test(); extern void bit_test(); -extern void can_router_test(); +extern void can_test(); extern void enum_test(); extern void i2c_util_test(); extern void input_pin_util_test(); @@ -44,7 +44,7 @@ int main() { hal::as_bytes_test(); hal::bit_test(); - hal::can_router_test(); + hal::can_test(); hal::enum_test(); hal::i2c_util_test(); hal::input_pin_util_test();