Clean up some code

Added comments, remove unused declarations and move around some
declarations and implementations to be more logic and tidy.

Change-Id: I4b392f9aa2471067b6d102532b620acc6afe3cb2
Signed-off-by: Loïc Collignon <loic.collignon@iot.bzh>

2 files changed, 209 insertions, 160 deletions

diff --git a/ahl-binding/ahl-binding.cpp b/ahl-binding/ahl-binding.cpp
index f3b1ae8..af1ae79 100644
--- a/ahl-binding/ahl-binding.cpp
+++ b/ahl-binding/ahl-binding.cpp
@@ -28,69 +28,104 @@ ahl_binding_t::ahl_binding_t()
 }
 
 /**
- * @brief Get the singleton instance.
- * @return The unique instance.
+ * @brief Creates static verbs.
  */
-ahl_binding_t& ahl_binding_t::instance()
+void ahl_binding_t::load_static_verbs()
 {
-	static ahl_binding_t s;
-	return s;
+	if (afb_api_add_verb(
+			handle_,
+			"get_roles",
+			"Retrieve array of available audio roles",
+			ahl_api_get_roles,
+			nullptr,
+			nullptr,
+			AFB_SESSION_NONE_X2, 0))
+	{
+		throw std::runtime_error("Failed to add 'get_role' verb to the API.");
+	}
+
+	if (afb_api_add_verb(
+			handle_,
+			"subscribe",
+			"Subscribe to \"volume_changed\" event",
+			ahl_api_subscribe,
+			nullptr,
+			nullptr,
+			AFB_SESSION_NONE_X2, 0))
+	{
+		throw std::runtime_error("Failed to add 'subscribe' verb to the API.");
+	}
+
+	if (afb_api_add_verb(
+			handle_,
+			"unsubscribe",
+			"Unsubscribe to \"volume_changed\" event",
+			ahl_api_unsubscribe,
+			nullptr,
+			nullptr,
+			AFB_SESSION_NONE_X2, 0))
+	{
+		throw std::runtime_error("Failed to add 'unsubscribe' verb to the API.");
+	}
 }
 
 /**
- * @brief This method is called during the pre-init phase of loading the binding.
- * @param[in] handle Handle to the api.
- * @return Status code, zero if success.
+ * @brief Find all controller's configuration files.
  */
-int ahl_binding_t::preinit(afb_api_t handle)
+void ahl_binding_t::load_controller_configs()
 {
-	handle_ = handle;
+	char* dir_list = getenv("CONTROL_CONFIG_PATH");
+	if (!dir_list) dir_list = strdup(CONTROL_CONFIG_PATH);
+	struct json_object* config_files = CtlConfigScan(dir_list, "policy");
+	if (!config_files) throw std::runtime_error("No config files found!");
 
-	try
+	// Only one file should be found this way, but read all just in case
+	size_t config_files_count = json_object_array_length(config_files);
+	for(int i = 0; i < config_files_count; ++i)
 	{
-		load_static_verbs();
-		load_controller_configs();
-
-		if (afb_api_on_event(handle_, ahl_api_on_event))
-			throw std::runtime_error("Failed to register event handler callback.");
+		config_entry_t file {json_object_array_get_idx(config_files, i)};
 
-		if (afb_api_on_init(handle_, ahl_api_init))
-			throw std::runtime_error("Failed to register init handler callback.");
-	}
-	catch(std::exception& e)
-	{
-		AFB_API_ERROR(handle, "%s", e.what());
-		return -1;
+		if(load_controller_config(file.filepath()) < 0)
+		{
+			std::stringstream ss;
+			ss  << "Failed to load config file '"
+				<< file.filename()
+				<< "' from '"
+				<< file.fullpath()
+				<< "'!";
+			throw std::runtime_error(ss.str());
+		}
 	}
-
-	return 0;
 }
 
 /**
- * @brief Initialize the API.
+ * @brief Load a controller's configuration.
+ * @param[in] path File to load.
+ * @return Zero on success, non-zero otherwise.
  */
-int ahl_binding_t::init()
+int ahl_binding_t::load_controller_config(const std::string& path)
 {
-	using namespace std::placeholders;
+	CtlConfigT* controller_config;
 
-	if (afb_api_require_api(handle_, HAL_MGR_API, 1))
+	controller_config = CtlLoadMetaData(handle_, path.c_str());
+	if (!controller_config)
 	{
-		AFB_API_ERROR(handle_, "Failed to require '%s' API!", HAL_MGR_API);
+		AFB_API_ERROR(handle_, "Failed to load controller from config file!");
 		return -1;
 	}
-	AFB_API_NOTICE(handle_, "Required '%s' API found!", HAL_MGR_API);
 
-	afb_api_seal(handle_);
-	AFB_API_NOTICE(handle_, "API is now sealed!");
+	static CtlSectionT controller_sections[] =
+	{
+		{.key = "plugins",  .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = PluginConfig,  .handle = nullptr, .actions = nullptr},
+		{.key = "onload",   .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = OnloadConfig,  .handle = nullptr, .actions = nullptr},
+		{.key = "controls", .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = ControlConfig, .handle = nullptr, .actions = nullptr},
+		{.key = "events",   .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = EventConfig,   .handle = nullptr, .actions = nullptr},
+		{.key = "roles",    .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = ahl_api_config_roles, .handle = nullptr, .actions = nullptr },
+		{.key = nullptr,    .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = nullptr,       .handle = nullptr, .actions = nullptr}
+	};
 
-    volume_changed_ = afb_api_make_event(handle_, "volume_changed");
-    if(!afb_event_is_valid(volume_changed_))
-    {
-        AFB_API_ERROR(handle_, "Failed to create the \"volume_changed\" event!");
-        return -2;
-    }
+	CtlLoadSections(handle_, controller_config, controller_sections);
 
-	if (update_streams()) return -1;
 	return 0;
 }
 
@@ -189,102 +224,148 @@ void ahl_binding_t::update_stream(std::string halname, std::string stream, std::
 	}
 }
 
-void ahl_binding_t::event(std::string name, json_object* arg)
+/**
+ * @brief Create a verb based on a role.
+ * @param[in] r Role to create a verb for.
+ * @return Zero on success, non-zero otherwise.
+ */
+int ahl_binding_t::create_api_verb(role_t* r)
 {
-	AFB_API_DEBUG(handle_, "Event '%s' received with the following arg: %s", name.c_str(), json_object_to_json_string(arg));
-}
+	AFB_API_NOTICE(handle_, "New audio role: %s", r->uid().c_str());
 
-void ahl_binding_t::load_static_verbs()
-{
 	if (afb_api_add_verb(
-			handle_,
-			"get_roles",
-			"Retrieve array of available audio roles",
-			ahl_api_get_roles,
-			nullptr,
-			nullptr,
-			AFB_SESSION_NONE_X2, 0))
+		handle_,
+		r->uid().c_str(),
+		r->description().c_str(),
+		ahl_api_role,
+		r,
+		nullptr,
+		AFB_SESSION_NONE_X2, 0))
 	{
-		throw std::runtime_error("Failed to add 'get_role' verb to the API.");
+		AFB_API_ERROR(handle_, "Failed to add '%s' verb to the API.",
+			r->uid().c_str());
+		return -1;
 	}
 
-	if (afb_api_add_verb(
-			handle_,
-			"subscribe",
-			"Subscribe to \"volume_changed\" event",
-			ahl_api_subscribe,
-			nullptr,
-			nullptr,
-			AFB_SESSION_NONE_X2, 0))
-	{
-		throw std::runtime_error("Failed to add 'subscribe' verb to the API.");
-	}
+	return 0;
+}
 
-	if (afb_api_add_verb(
-			handle_,
-			"unsubscribe",
-			"Unsubscribe to \"volume_changed\" event",
-			ahl_api_unsubscribe,
-			nullptr,
-			nullptr,
-			AFB_SESSION_NONE_X2, 0))
-	{
-		throw std::runtime_error("Failed to add 'unsubscribe' verb to the API.");
-	}
+/**
+ * @brief Get the singleton instance.
+ * @return The unique instance.
+ */
+ahl_binding_t& ahl_binding_t::instance()
+{
+	static ahl_binding_t s;
+	return s;
 }
 
-void ahl_binding_t::load_controller_configs()
+/**
+ * @brief Get the api handle.
+ * @return The api handle.
+ */
+afb_api_t ahl_binding_t::handle() const
 {
-	char* dir_list = getenv("CONTROL_CONFIG_PATH");
-	if (!dir_list) dir_list = strdup(CONTROL_CONFIG_PATH);
-	struct json_object* config_files = CtlConfigScan(dir_list, "policy");
-	if (!config_files) throw std::runtime_error("No config files found!");
+	return handle_;
+}
 
-	// Only one file should be found this way, but read all just in case
-	size_t config_files_count = json_object_array_length(config_files);
-	for(int i = 0; i < config_files_count; ++i)
+/**
+ * @brief Get the role list.
+ * @return A vector of roles.
+ */
+const std::vector<role_t> ahl_binding_t::roles() const
+{
+	return roles_;
+}
+
+
+/**
+ * @brief This method is called during the pre-init phase of loading the binding.
+ * @param[in] handle Handle to the api.
+ * @return Status code, zero if success.
+ */
+int ahl_binding_t::preinit(afb_api_t handle)
+{
+	handle_ = handle;
+
+	try
 	{
-		config_entry_t file {json_object_array_get_idx(config_files, i)};
+		load_static_verbs();
+		load_controller_configs();
 
-		if(load_controller_config(file.filepath()) < 0)
-		{
-			std::stringstream ss;
-			ss  << "Failed to load config file '"
-				<< file.filename()
-				<< "' from '"
-				<< file.fullpath()
-				<< "'!";
-			throw std::runtime_error(ss.str());
-		}
+		if (afb_api_on_event(handle_, ahl_api_on_event))
+			throw std::runtime_error("Failed to register event handler callback.");
+
+		if (afb_api_on_init(handle_, ahl_api_init))
+			throw std::runtime_error("Failed to register init handler callback.");
+	}
+	catch(std::exception& e)
+	{
+		AFB_API_ERROR(handle, "%s", e.what());
+		return -1;
 	}
+
+	return 0;
 }
 
-int ahl_binding_t::load_controller_config(const std::string& path)
+/**
+ * @brief Initialize the API.
+ */
+int ahl_binding_t::init()
 {
-	CtlConfigT* controller_config;
+	using namespace std::placeholders;
 
-	controller_config = CtlLoadMetaData(handle_, path.c_str());
-	if (!controller_config)
+	if (afb_api_require_api(handle_, HAL_MGR_API, 1))
 	{
-		AFB_API_ERROR(handle_, "Failed to load controller from config file!");
+		AFB_API_ERROR(handle_, "Failed to require '%s' API!", HAL_MGR_API);
 		return -1;
 	}
+	AFB_API_NOTICE(handle_, "Required '%s' API found!", HAL_MGR_API);
 
-	static CtlSectionT controller_sections[] =
-	{
-		{.key = "plugins",  .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = PluginConfig,  .handle = nullptr, .actions = nullptr},
-		{.key = "onload",   .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = OnloadConfig,  .handle = nullptr, .actions = nullptr},
-		{.key = "controls", .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = ControlConfig, .handle = nullptr, .actions = nullptr},
-		{.key = "events",   .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = EventConfig,   .handle = nullptr, .actions = nullptr},
-		{.key = "roles",    .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = ahl_api_config_roles, .handle = nullptr, .actions = nullptr },
-		{.key = nullptr,    .uid = nullptr, .info = nullptr, .prefix = nullptr, .loadCB = nullptr,       .handle = nullptr, .actions = nullptr}
-	};
+	afb_api_seal(handle_);
+	AFB_API_NOTICE(handle_, "API is now sealed!");
 
-	CtlLoadSections(handle_, controller_config, controller_sections);
+	volume_changed_ = afb_api_make_event(handle_, "volume_changed");
+	if(!afb_event_is_valid(volume_changed_))
+	{
+		AFB_API_ERROR(handle_, "Failed to create the \"volume_changed\" event!");
+		return -2;
+	}
 
+	if (update_streams()) return -1;
 	return 0;
 }
 
+/**
+ * @brief Called when an event is received by this binding.
+ * @param[in] name Event's name.
+ * @param[in] arg Event's argument.
+ */
+void ahl_binding_t::event(std::string name, json_object* arg)
+{
+	AFB_API_DEBUG(handle_, "Event '%s' received with the following arg: %s", name.c_str(), json_object_to_json_string(arg));
+}
+
+/**
+ * @brief Emit the 'volume-changed' event.
+ * @param[in] role Role's uid.
+ * @param[in] volume New volume.
+ * @return Zero on success, non-zero otherwise.
+ */
+int ahl_binding_t::emit_volume_changed(const std::string& role, int volume) const
+{
+	json_object* data = json_object_new_object();
+	json_object_object_add(data, "role", json_object_new_string(role.c_str()));
+	json_object_object_add(data, "volume", json_object_new_int(volume));
+
+	return afb_event_push(volume_changed_, data);
+}
+
+/**
+ * @brief Parse the role configuration section.
+ * @param[in] o Role's section.
+ * @return Zero on success, non-zero otherwise.
+ */
 int ahl_binding_t::parse_roles_config(json_object* o)
 {
 	assert(o != nullptr);
@@ -308,27 +389,12 @@ int ahl_binding_t::parse_roles_config(json_object* o)
 	return 0;
 }
 
-int ahl_binding_t::create_api_verb(role_t* r)
-{
-	AFB_API_NOTICE(handle_, "New audio role: %s", r->uid().c_str());
-
-	if (afb_api_add_verb(
-		handle_,
-		r->uid().c_str(),
-		r->description().c_str(),
-		ahl_api_role,
-		r,
-		nullptr,
-		AFB_SESSION_NONE_X2, 0))
-	{
-		AFB_API_ERROR(handle_, "Failed to add '%s' verb to the API.",
-			r->uid().c_str());
-		return -1;
-	}
-
-	return 0;
-}
+// ---------- Callbacks to verbs ----------
 
+/**
+ * @brief Get the roles list.
+ * @param[in] req Request to answer.
+ */
 void ahl_binding_t::get_roles(afb_req_t req) const
 {
 	json_bool verbose = FALSE;
@@ -349,6 +415,10 @@ void ahl_binding_t::get_roles(afb_req_t req) const
 	afb_req_success(req, result, nullptr);
 }
 
+/**
+ * @brief Subscribe to events.
+ * @param[in] req Request to answer.
+ */
 void ahl_binding_t::subscribe(afb_req_t req) const
 {
     if (afb_req_subscribe(req, volume_changed_))
@@ -356,7 +426,10 @@ void ahl_binding_t::subscribe(afb_req_t req) const
     else
         afb_req_success(req, nullptr, "Subscribed to \"volume_changed\" event!");
 }
-
+/**
+ * @brief Unsubscribe to events.
+ * @param[in] req Request to answer.
+ */
 void ahl_binding_t::unsubscribe(afb_req_t req) const
 {
     if (afb_req_unsubscribe(req, volume_changed_))
@@ -364,22 +437,3 @@ void ahl_binding_t::unsubscribe(afb_req_t req) const
     else
         afb_req_success(req, nullptr, "Unsubscribed from \"volume_changed\" event!");
 }
-
-int ahl_binding_t::emit_volume_changed(const std::string& role, int volume) const
-{
-    json_object* data = json_object_new_object();
-    json_object_object_add(data, "role", json_object_new_string(role.c_str()));
-    json_object_object_add(data, "volume", json_object_new_int(volume));
-
-    return afb_event_push(volume_changed_, data);
-}
-
-const std::vector<role_t> ahl_binding_t::roles() const
-{
-	return roles_;
-}
-
-afb_api_t ahl_binding_t::handle() const
-{
-	return handle_;
-}
diff --git a/ahl-binding/ahl-binding.hpp b/ahl-binding/ahl-binding.hpp
index 9e670bb..881d906 100644
--- a/ahl-binding/ahl-binding.hpp
+++ b/ahl-binding/ahl-binding.hpp
@@ -46,31 +46,26 @@ private:
 	explicit ahl_binding_t();
 
 	void load_static_verbs();
-
-
 	void load_controller_configs();
 	int load_controller_config(const std::string& path);
 	int update_streams();
 	void update_stream(std::string hal, std::string stream, std::string deviceuri);
 	int create_api_verb(role_t* r);
 
-	void policy_open(afb_req_t req, const role_t& role);
-
 public:
 	static ahl_binding_t& instance();
+	afb_api_t handle() const;
+	const std::vector<role_t> roles() const;
+
 	int preinit(afb_api_t handle);
 	int init();
 	void event(std::string name, json_object* arg);
+	int emit_volume_changed(const std::string& role, int volume) const;
+	int parse_roles_config(json_object* o);
+
 	void get_roles(afb_req_t req) const;
 	void subscribe(afb_req_t req) const;
 	void unsubscribe(afb_req_t req) const;
-	int emit_volume_changed(const std::string& role, int volume) const;
-
-	const std::vector<role_t> roles() const;
-	afb_api_t handle() const;
-
-	void audiorole(afb_req_t req);
-	int parse_roles_config(json_object* o);
 };