🚧 Apply suggestions from code review + simplify some part of the code

leka · Apr 12, 2022 · 24c7b92 · 24c7b92
1 parent 43ec72c
commit 24c7b92
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 35 deletions.
diff --git 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 &params) final
 	{
 		if (params.handle == apply_update_characteristic.getValueHandle()) {
-			apply_update_value = static_cast<bool>(params.data[0]);
-			if (apply_update_value && _on_apply_update_true) {
-				_on_apply_update_true();
+			auto must_apply_update = static_cast<bool>(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<void()> &callback) { _on_apply_update_true = callback; }
+	void onApplyUpdate(const std::function<void()> &callback) { _on_apply_update_callback = callback; }
 
   private:
-	bool apply_update_value {false};
+	bool default_apply_update {false};
 	WriteOnlyGattCharacteristic<bool> apply_update_characteristic {
-		service::firmware_update::characteristic::apply_update, &apply_update_value};
-	std::function<void()> _on_apply_update_true {};
+		service::firmware_update::characteristic::apply_update, &default_apply_update};
+	std::function<void()> _on_apply_update_callback {};
 
 	FirmwareVersion version {};
 

diff --git 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<void()> 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<uint8_t>(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<void()> 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<uint8_t>(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<uint8_t>(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
@@ -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
@@ -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);
 	}
 }