From 24c7b92c2e93c7b905628ff5a5a470b776e51202 Mon Sep 17 00:00:00 2001 From: Yann Locatelli Date: Tue, 12 Apr 2022 22:30:55 +0200 Subject: [PATCH] :construction: Apply suggestions from code review + simplify some part of the code --- libs/BLEKit/include/BLEServiceUpdate.h | 16 +++++------ libs/BLEKit/tests/BLEServiceUpdate_test.cpp | 31 +++++++-------------- libs/RobotKit/include/RobotController.h | 2 +- spikes/lk_ble/main.cpp | 5 +--- 4 files changed, 19 insertions(+), 35 deletions(-) diff --git a/libs/BLEKit/include/BLEServiceUpdate.h b/libs/BLEKit/include/BLEServiceUpdate.h index 139baff2e6..5f542cbf69 100644 --- a/libs/BLEKit/include/BLEServiceUpdate.h +++ b/libs/BLEKit/include/BLEServiceUpdate.h @@ -15,16 +15,14 @@ class BLEServiceUpdate : public interface::BLEService public: BLEServiceUpdate() : interface::BLEService(service::firmware_update::uuid, _characteristic_table) {} - auto getApplyUpdateValue() const -> bool { return apply_update_value; } - auto getVersion() const -> FirmwareVersion { return version; } void onDataReceived(const data_received_handle_t ¶ms) final { if (params.handle == apply_update_characteristic.getValueHandle()) { - apply_update_value = static_cast(params.data[0]); - if (apply_update_value && _on_apply_update_true) { - _on_apply_update_true(); + auto must_apply_update = static_cast(params.data[0]); + if (must_apply_update && _on_apply_update_callback) { + _on_apply_update_callback(); } } if (params.handle == version_major_characteristic.getValueHandle()) { @@ -38,13 +36,13 @@ class BLEServiceUpdate : public interface::BLEService } } - void onApplyUpdateTrue(const std::function &callback) { _on_apply_update_true = callback; } + void onApplyUpdate(const std::function &callback) { _on_apply_update_callback = callback; } private: - bool apply_update_value {false}; + bool default_apply_update {false}; WriteOnlyGattCharacteristic apply_update_characteristic { - service::firmware_update::characteristic::apply_update, &apply_update_value}; - std::function _on_apply_update_true {}; + service::firmware_update::characteristic::apply_update, &default_apply_update}; + std::function _on_apply_update_callback {}; FirmwareVersion version {}; diff --git a/libs/BLEKit/tests/BLEServiceUpdate_test.cpp b/libs/BLEKit/tests/BLEServiceUpdate_test.cpp index c2601d597e..a1dbff7120 100644 --- a/libs/BLEKit/tests/BLEServiceUpdate_test.cpp +++ b/libs/BLEKit/tests/BLEServiceUpdate_test.cpp @@ -37,37 +37,30 @@ TEST_F(BLEServiceUpdateTest, initialisation) EXPECT_NE(&service_update, nullptr); } -TEST_F(BLEServiceUpdateTest, getApplyUpdateValueDefault) -{ - auto actual_apply_update_value = service_update.getApplyUpdateValue(); - EXPECT_EQ(actual_apply_update_value, default_apply_update_value); -} - -TEST_F(BLEServiceUpdateTest, getApplyUpdateValueTrue) +TEST_F(BLEServiceUpdateTest, getApplyUpdateValueTrueCallbackSet) { bool expected_apply_update_value = true; + testing::MockFunction mock_callback; + service_update.onApplyUpdate(mock_callback.AsStdFunction()); + + EXPECT_CALL(mock_callback, Call).Times(1); + auto convert_to_handle_data_type = [](bool value) { return std::make_shared(value).get(); }; onDataReceivedProcess(convert_to_handle_data_type(expected_apply_update_value)); - - auto actual_apply_update_value = service_update.getApplyUpdateValue(); - EXPECT_EQ(actual_apply_update_value, expected_apply_update_value); } -TEST_F(BLEServiceUpdateTest, getApplyUpdateValueTrueCallbackSet) +TEST_F(BLEServiceUpdateTest, getApplyUpdateValueFalseCallbackSet) { - bool expected_apply_update_value = true; + bool expected_apply_update_value = false; testing::MockFunction mock_callback; - service_update.onApplyUpdateTrue(mock_callback.AsStdFunction()); + service_update.onApplyUpdate(mock_callback.AsStdFunction()); - EXPECT_CALL(mock_callback, Call); + EXPECT_CALL(mock_callback, Call).Times(0); auto convert_to_handle_data_type = [](bool value) { return std::make_shared(value).get(); }; onDataReceivedProcess(convert_to_handle_data_type(expected_apply_update_value)); - - auto actual_apply_update_value = service_update.getApplyUpdateValue(); - EXPECT_EQ(actual_apply_update_value, expected_apply_update_value); } TEST_F(BLEServiceUpdateTest, getApplyUpdateValueNotSameHandle) @@ -79,10 +72,6 @@ TEST_F(BLEServiceUpdateTest, getApplyUpdateValueNotSameHandle) auto convert_to_handle_data_type = [](bool value) { return std::make_shared(value).get(); }; onDataReceivedProcess(convert_to_handle_data_type(sent_value)); - - auto actual_apply_update_value = service_update.getApplyUpdateValue(); - EXPECT_EQ(actual_apply_update_value, expected_apply_update_value); - EXPECT_NE(actual_apply_update_value, sent_value); } TEST_F(BLEServiceUpdateTest, getVersionMajorDefault) diff --git a/libs/RobotKit/include/RobotController.h b/libs/RobotKit/include/RobotController.h index beea115b94..7437fd314f 100644 --- a/libs/RobotKit/include/RobotController.h +++ b/libs/RobotKit/include/RobotController.h @@ -220,7 +220,7 @@ class RobotController : public interface::RobotController _service_commands.onCommandsReceived(on_commands_received); auto on_update_requested = [this]() { raise(event::update_requested {}); }; - _service_update.onApplyUpdateTrue(on_update_requested); + _service_update.onApplyUpdate(on_update_requested); raise(event::setup_complete {}); } diff --git a/spikes/lk_ble/main.cpp b/spikes/lk_ble/main.cpp index 530c71de33..c489c6c400 100644 --- a/spikes/lk_ble/main.cpp +++ b/spikes/lk_ble/main.cpp @@ -54,9 +54,6 @@ auto main() -> int auto version = service_update.getVersion(); - auto apply_update = service_update.getApplyUpdateValue(); - - log_info("Requested version: %d.%d.%d | Apply Update? %d", version.major, version.minor, version.revision, - apply_update); + log_info("Requested version: %d.%d.%d", version.major, version.minor, version.revision); } }