Cleanup of output format / device type handling.

src/applib/selftest.cpp

@@ -19,7 +19,7 @@ License: GNU General Public License v3.0 only
 #include "smartctl_text_ata_parser.h"
 #include "selftest.h"
 #include "ata_storage_property_descr.h"
-
+#include "smartctl_version_parser.h"
 
 
 
@@ -243,7 +243,7 @@ std::string SelfTest::update(const std::shared_ptr<CommandExecutor>& smartctl_ex
 		return error_msg;
 
 	const AtaStorageAttribute::DiskType disk_type = drive_->get_is_hdd() ? AtaStorageAttribute::DiskType::Hdd : AtaStorageAttribute::DiskType::Ssd;
-	auto parser = SmartctlParser::create(SmartctlParserType::TextAta);
+	auto parser = SmartctlParser::create(SmartctlParserType::Ata, SmartctlVersionParser::get_default_format(SmartctlParserType::Ata));
 	DBG_ASSERT_RETURN(parser, "Cannot create parser");
 
 	auto parse_status = parser->parse(output);

src/applib/smartctl_json_ata_parser.cpp

@@ -153,7 +153,7 @@ hz::ExpectedVoid<SmartctlParserError> SmartctlJsonAtaParser::parse_version(const
 		p.section = AtaStorageProperty::Section::info;  // add to info section
 		add_property(p);
 	}
-	if (!SmartctlVersionParser::check_parsed_version(SmartctlParserType::JsonAta, smartctl_version)) {
+	if (!SmartctlVersionParser::check_format_supported(SmartctlOutputFormat::Json, smartctl_version)) {
 		debug_out_warn("app", DBG_FUNC_MSG << "Incompatible smartctl version. Returning.\n");
 		return hz::Unexpected(SmartctlParserError::IncompatibleVersion, "Incompatible smartctl version.");
 	}

src/applib/smartctl_parser.cpp

@@ -21,39 +21,50 @@ License: GNU General Public License v3.0 only
 
 
 
-std::unique_ptr<SmartctlParser> SmartctlParser::create(SmartctlParserType type)
+std::unique_ptr<SmartctlParser> SmartctlParser::create(SmartctlParserType type, SmartctlOutputFormat format)
 {
 	switch(type) {
-		case SmartctlParserType::JsonBasic:
-			return std::make_unique<SmartctlJsonBasicParser>();
+		case SmartctlParserType::Basic:
+			switch(format) {
+				case SmartctlOutputFormat::Json:
+					return std::make_unique<SmartctlJsonBasicParser>();
+					break;
+				case SmartctlOutputFormat::Text:
+					return std::make_unique<SmartctlTextBasicParser>();
+					break;
+			}
 			break;
-		case SmartctlParserType::JsonAta:
-			return std::make_unique<SmartctlJsonAtaParser>();
+		case SmartctlParserType::Ata:
+			switch(format) {
+				case SmartctlOutputFormat::Json:
+					return std::make_unique<SmartctlJsonAtaParser>();
+					break;
+				case SmartctlOutputFormat::Text:
+					return std::make_unique<SmartctlTextAtaParser>();
+					break;
+			}
 			break;
-		case SmartctlParserType::TextBasic:
-			return std::make_unique<SmartctlTextBasicParser>();
-			break;
-		case SmartctlParserType::TextAta:
-			return std::make_unique<SmartctlTextAtaParser>();
+		case SmartctlParserType::Nvme:
+			// TODO
 			break;
 	}
 	return nullptr;
 }
 
 
 
-hz::ExpectedValue<SmartctlParserFormat, SmartctlParserError> SmartctlParser::detect_output_format(std::string_view smartctl_output)
+hz::ExpectedValue<SmartctlOutputFormat, SmartctlParserError> SmartctlParser::detect_output_format(std::string_view smartctl_output)
 {
 	// Look for the first non-whitespace symbol
 	const auto* first_symbol = std::find_if(smartctl_output.begin(), smartctl_output.end(), [&](char c) {
 		return !std::isspace(c, std::locale::classic());
 	});
 	if (first_symbol != smartctl_output.end()) {
 		if (*first_symbol == '{') {
-			return SmartctlParserFormat::Json;
+			return SmartctlOutputFormat::Json;
 		}
 		if (smartctl_output.rfind("smartctl", static_cast<std::size_t>(first_symbol - smartctl_output.begin())) == 0) {
-			return SmartctlParserFormat::Text;
+			return SmartctlOutputFormat::Text;
 		}
 		return hz::Unexpected(SmartctlParserError::UnsupportedFormat, "Unsupported format while trying to detect smartctl output format.");
 	}

src/applib/smartctl_parser.h

@@ -66,7 +66,7 @@ class SmartctlParser {
 
 		/// Create an instance of this class.
 		/// \return nullptr if no such class exists
-		static std::unique_ptr<SmartctlParser> create(SmartctlParserType type);
+		static std::unique_ptr<SmartctlParser> create(SmartctlParserType type, SmartctlOutputFormat format);
 
 
 		/// Parse full "smartctl -x" output.
@@ -75,7 +75,7 @@ class SmartctlParser {
 
 
 		/// Detect smartctl output type (text, json).
-		[[nodiscard]] static hz::ExpectedValue<SmartctlParserFormat, SmartctlParserError> detect_output_format(std::string_view smartctl_output);
+		[[nodiscard]] static hz::ExpectedValue<SmartctlOutputFormat, SmartctlParserError> detect_output_format(std::string_view smartctl_output);
 
 
 		/// Get parsed properties.

src/applib/smartctl_parser_types.h

@@ -19,45 +19,22 @@ License: GNU General Public License v3.0 only
 
 
 enum class SmartctlParserType {
-	JsonBasic,  ///< Info only
-	JsonAta,
-	TextBasic,  ///< Info only
-	TextAta,
+	Basic,  ///< Info only, supports all types of devices
+	Ata,  ///< (S)ATA
+	Nvme,  ///< NVMe
+//	Scsi,  ///< SCSI
 };
 
 
 
-/// Helper structure for enum-related functions
-struct SmartctlParserTypeExt
-		: public hz::EnumHelper<
-				SmartctlParserType,
-				SmartctlParserTypeExt,
-		        Glib::ustring>
-{
-	static constexpr inline SmartctlParserType default_value = SmartctlParserType::JsonAta;
-
-	static std::unordered_map<EnumType, std::pair<std::string, Glib::ustring>> build_enum_map()
-	{
-		return {
-				{SmartctlParserType::JsonBasic, {"json_basic", _("JSON Basic")}},
-				{SmartctlParserType::JsonAta, {"json_ata", _("JSON ATA")}},
-				{SmartctlParserType::TextBasic, {"text_basic", _("Text Basic")}},
-				{SmartctlParserType::TextAta, {"text_ata", _("Text ATA")}},
-		};
-	}
-
-};
-
-
-
-enum class SmartctlParserFormat {
+enum class SmartctlOutputFormat {
 	Json,
 	Text,
 };
 
 
 
-enum class SmartctlParserSettingType {
+enum class SmartctlParserPreferenceType {
 	Auto,
 	Json,
 	Text,
@@ -66,20 +43,20 @@ enum class SmartctlParserSettingType {
 
 
 /// Helper structure for enum-related functions
-struct SmartctlParserSettingTypeExt
+struct SmartctlParserPreferenceTypeExt
 		: public hz::EnumHelper<
-				SmartctlParserSettingType,
-				SmartctlParserSettingTypeExt,
+				SmartctlParserPreferenceType,
+				SmartctlParserPreferenceTypeExt,
 		        Glib::ustring>
 {
-	static constexpr inline SmartctlParserSettingType default_value = SmartctlParserSettingType::Auto;
+	static constexpr inline SmartctlParserPreferenceType default_value = SmartctlParserPreferenceType::Auto;
 
 	static std::unordered_map<EnumType, std::pair<std::string, Glib::ustring>> build_enum_map()
 	{
 		return {
-			{SmartctlParserSettingType::Auto, {"auto", _("Automatic")}},
-			{SmartctlParserSettingType::Json, {"json", _("JSON")}},
-			{SmartctlParserSettingType::Text, {"text", _("Text")}},
+			{SmartctlParserPreferenceType::Auto, {"auto", _("Automatic")}},
+			{SmartctlParserPreferenceType::Json, {"json", _("JSON")}},
+			{SmartctlParserPreferenceType::Text, {"text", _("Text")}},
 		};
 	}
 

src/applib/smartctl_text_ata_parser.cpp

@@ -223,7 +223,7 @@ hz::ExpectedVoid<SmartctlParserError> SmartctlTextAtaParser::parse(std::string_v
 		add_property(p);
 	}
 
-	if (!SmartctlVersionParser::check_parsed_version(SmartctlParserType::TextAta, version)) {
+	if (!SmartctlVersionParser::check_format_supported(SmartctlOutputFormat::Text, version)) {
 		debug_out_warn("app", DBG_FUNC_MSG << "Incompatible smartctl version. Returning.\n");
 		return hz::Unexpected(SmartctlParserError::IncompatibleVersion, "Incompatible smartctl version.");
 	}

src/applib/smartctl_version_parser.cpp

@@ -53,31 +53,32 @@ std::optional<double> SmartctlVersionParser::get_numeric_version(const std::stri
 
 
 
-bool SmartctlVersionParser::check_parsed_version(SmartctlParserType parser_type, const std::string& version_only)
+bool SmartctlVersionParser::check_format_supported(SmartctlOutputFormat format, const std::string& version_only)
 {
 	if (auto numeric_version = get_numeric_version(version_only); numeric_version.has_value()) {
-		switch(parser_type) {
-			case SmartctlParserType::JsonBasic:
-			case SmartctlParserType::JsonAta:
-				return numeric_version.value() >= minimum_req_json_version;
-			case SmartctlParserType::TextBasic:
-			case SmartctlParserType::TextAta:
+		switch(format) {
+			case SmartctlOutputFormat::Text:
 				return numeric_version.value() >= minimum_req_text_version;
+			case SmartctlOutputFormat::Json:
+				return numeric_version.value() >= minimum_req_json_version;
 		}
 	}
 	return false;
 }
 
 
 
-std::optional<SmartctlParserType> SmartctlVersionParser::detect_supported_parser_type(const std::string& version_only)
+SmartctlOutputFormat SmartctlVersionParser::get_default_format(SmartctlParserType parser_type)
 {
-	for (auto type : SmartctlParserTypeExt::getAllValues()) {
-		if (check_parsed_version(type, version_only)) {
-			return type;
-		}
+	switch (parser_type) {
+		case SmartctlParserType::Basic:
+			return SmartctlOutputFormat::Json;
+		case SmartctlParserType::Ata:
+			return SmartctlOutputFormat::Json;
+		case SmartctlParserType::Nvme:
+			return SmartctlOutputFormat::Json;
 	}
-	return std::nullopt;
+	return SmartctlOutputFormat::Json;
 }
 
 

src/applib/smartctl_version_parser.h

@@ -42,11 +42,11 @@ class SmartctlVersionParser {
 
 
 		/// Check that the version of smartctl output can be parsed with a parser.
-		static bool check_parsed_version(SmartctlParserType parser_type, const std::string& version_only);
+		static bool check_format_supported(SmartctlOutputFormat format, const std::string& version_only);
 
 
-		/// Detect smartctl parser type based on smartctl version
-		static std::optional<SmartctlParserType> detect_supported_parser_type(const std::string& version_only);
+		/// Get default output format for a parser type.
+		static SmartctlOutputFormat get_default_format(SmartctlParserType parser_type);
 
 
 	private:

src/applib/storage_device.cpp

@@ -23,7 +23,7 @@ License: GNU General Public License v3.0 only
 #include "storage_settings.h"
 #include "smartctl_executor.h"
 #include "smartctl_version_parser.h"
-#include "smartctl_text_parser_helper.h"
+//#include "smartctl_text_parser_helper.h"
 #include "ata_storage_property_descr.h"
 
 
@@ -143,7 +143,7 @@ std::string StorageDevice::parse_basic_data(bool do_set_properties, bool emit_si
 	AtaStorageAttribute::DiskType disk_type = AtaStorageAttribute::DiskType::Any;
 
 	// Try the basic parser first. If it succeeds, use the specialized parser.
-	auto basic_parser = SmartctlParser::create(SmartctlParserType::TextBasic);
+	auto basic_parser = SmartctlParser::create(SmartctlParserType::Basic, SmartctlOutputFormat::Json);
 	DBG_ASSERT_RETURN(basic_parser, "Cannot create parser");
 
 	auto parse_status = basic_parser->parse(this->get_info_output());
@@ -200,7 +200,7 @@ std::string StorageDevice::parse_basic_data(bool do_set_properties, bool emit_si
 	// Note that this may try to parse data the second time (it may already have
 	// been parsed by parse_data() which failed at it).
 	if (do_set_properties) {
-		auto parser = SmartctlParser::create(SmartctlParserType::TextAta);
+		auto parser = SmartctlParser::create(SmartctlParserType::Ata, SmartctlOutputFormat::Json);
 		DBG_ASSERT_RETURN(parser, "Cannot create parser");
 
 		if (parser->parse(this->info_output_)) {  // try to parse it
@@ -230,19 +230,25 @@ std::string StorageDevice::fetch_data_and_parse(const std::shared_ptr<CommandExe
 	std::string output;
 	std::string error_msg;
 
-	const SmartctlParserSettingType default_parser_type = SmartctlParserSettingType::Json;
-
 	// instead of -x, we use all the individual options -x encompasses, so that
 	// an addition to default -x output won't affect us.
 	if (this->get_type_argument() == "scsi") {  // not sure about correctness... FIXME probably fails with RAID/scsi
+		const auto default_parser_type = SmartctlVersionParser::get_default_format(SmartctlParserType::Basic);
 		// This doesn't do much yet, but just in case...
 		// SCSI equivalent of -x:
-		error_msg = execute_device_smartctl("--health --info --attributes --log=error --log=selftest --log=background --log=sasphy", smartctl_ex, output);
+		std::string command_options = "--health --info --attributes --log=error --log=selftest --log=background --log=sasphy";
+		if (default_parser_type == SmartctlOutputFormat::Json) {
+			// --json flags: o means include original output (just in case).
+			command_options += " --json=o";
+		}
+
+		error_msg = execute_device_smartctl(command_options, smartctl_ex, output);
 
 	} else {
+		const auto default_parser_type = SmartctlVersionParser::get_default_format(SmartctlParserType::Ata);
 		// ATA equivalent of -x.
 		std::string command_options = "--health --info --get=all --capabilities --attributes --format=brief --log=xerror,50,error --log=xselftest,50,selftest --log=selective --log=directory --log=scttemp --log=scterc --log=devstat --log=sataphy";
-		if (default_parser_type == SmartctlParserSettingType::Json) {
+		if (default_parser_type == SmartctlOutputFormat::Json) {
 			// --json flags: o means include original output (just in case).
 			command_options += " --json=o";
 		}
@@ -284,12 +290,9 @@ std::string StorageDevice::parse_data()
 	}
 
 	// TODO Choose format according to device type
-	SmartctlParserType parser_type = SmartctlParserType::TextAta;
-	if (parser_format == SmartctlParserFormat::Json) {
-		parser_type = SmartctlParserType::JsonAta;
-	}
+	SmartctlParserType parser_type = SmartctlParserType::Ata;
 
-	auto parser = SmartctlParser::create(parser_type);
+	auto parser = SmartctlParser::create(parser_type, parser_format.value());
 	DBG_ASSERT_RETURN(parser, "Cannot create parser");
 
 	// Try to parse it (parse only, set the properties after basic parsing).

src/applib/tests/test_smartctl_parser.cpp

@@ -25,17 +25,17 @@ TEST_CASE("SmartctlFormatDetection", "[app][parser]")
 
 	REQUIRE(SmartctlParser::detect_output_format("smart").error().data() == SmartctlParserError::UnsupportedFormat);
 
-	REQUIRE(SmartctlParser::detect_output_format("{  }").value() == SmartctlParserFormat::Json);
+	REQUIRE(SmartctlParser::detect_output_format("{  }").value() == SmartctlOutputFormat::Json);
 
-	REQUIRE(SmartctlParser::detect_output_format(" \n {  } ").value() == SmartctlParserFormat::Json);
+	REQUIRE(SmartctlParser::detect_output_format(" \n {  } ").value() == SmartctlOutputFormat::Json);
 
-	REQUIRE(SmartctlParser::detect_output_format("smartctl").value() == SmartctlParserFormat::Text);
+	REQUIRE(SmartctlParser::detect_output_format("smartctl").value() == SmartctlOutputFormat::Text);
 
 	REQUIRE(SmartctlParser::detect_output_format(
 R"(smartctl 7.2 2020-12-30 r5155 [x86_64-linux-5.3.18-lp152.66-default] (SUSE RPM)
 Copyright (C) 2002-20, Bruce Allen, Christian Franke, www.smartmontools.org
 
-)").value() == SmartctlParserFormat::Text);
+)").value() == SmartctlOutputFormat::Text);
 
 }