chore(config): disallow leading zeroes in firmware versions (#1568)

zwave-js · Feb 3, 2021 · fc4e9b1 · fc4e9b1
1 parent e3bd592
commit fc4e9b1
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 47 deletions.
diff --git a/docs/development/config-files.md b/docs/development/config-files.md
@@ -51,7 +51,10 @@ Since different firmware versions of a device may have different config params,
 ```
 
 The default `min` version is `0.0` and the default `max` version is `255.255`.
-All other firmware ranges should be reflected in the filename.
+All other firmware ranges should be reflected in the filename. This also means that `0.0-` and `-255.255` should not be part of the filename, as they are implied.
+
+> [!NOTE]
+> Although some manufacturers tend to display firmware versions with leading zeroes, firmwares are interpreted as two numbers. This means `2.01` is equivalent to `2.1`. Leading zeroes **must not** be used in config files to avoid confusion.
 
 ### `metadata`
 

diff --git a/maintenance/tools.ts b/maintenance/tools.ts
@@ -48,11 +48,10 @@ export function reportProblem({
 			}::${message.replace(/\n/g, "%0A")}\n`,
 		);
 	} else {
+		console[severity](`${filename}${line != undefined ? `:${line}` : ""}:`);
 		console[severity](
 			(severity === "warn" ? yellow : red)(
-				`${filename}${
-					line != undefined ? `:${line}` : ""
-				}:\n[${severity.toUpperCase()}] ${message}\n`,
+				`[${severity.toUpperCase()}] ${message}\n`,
 			),
 		);
 	}

diff --git a/packages/config/maintenance/lintConfigFiles.ts b/packages/config/maintenance/lintConfigFiles.ts
@@ -439,52 +439,64 @@ Did you mean to use ${opt.value >>> shiftAmount}?`,
 		}
 
 		// Validate firmware versions
-		if (typeof config.firmwareVersion === "boolean") {
-			if (config.firmwareVersion !== false) {
+
+		if (config.firmwareVersion.max === "255.0") {
+			addWarning(
+				file,
+				`The maximum firmware version is 255.0. Did you mean 255.255?`,
+			);
+		} else {
+			// Check for invalid version parts
+			const [minMajor, minMinor] = config.firmwareVersion.min
+				.split(".", 2)
+				.map((v) => parseInt(v, 10));
+			if (
+				minMajor < 0 ||
+				minMajor > 255 ||
+				minMinor < 0 ||
+				minMinor > 255
+			) {
 				addError(
 					file,
-					`Invalid firmware version ${
-						config.firmwareVersion as any
-					}. The firmware version must be an object with "min" and "max" properties or false.`,
+					`The minimum firmware version ${config.firmwareVersion.min} is invalid. Each version part must be between 0 and 255.`,
 				);
 			}
-		} else {
-			if (config.firmwareVersion.max === "255.0") {
-				addWarning(
+
+			const [maxMajor, maxMinor] = config.firmwareVersion.max
+				.split(".", 2)
+				.map((v) => parseInt(v, 10));
+			if (
+				maxMajor < 0 ||
+				maxMajor > 255 ||
+				maxMinor < 0 ||
+				maxMinor > 255
+			) {
+				addError(
 					file,
-					`The maximum firmware version is 255.0. Did you mean 255.255?`,
+					`The maximum firmware version ${config.firmwareVersion.max} is invalid. Each version part must be between 0 and 255.`,
 				);
-			} else {
-				// Check for invalid version parts
-				const [minMajor, minMinor] = config.firmwareVersion.min
-					.split(".", 2)
-					.map((v) => parseInt(v, 10));
-				if (
-					minMajor < 0 ||
-					minMajor > 255 ||
-					minMinor < 0 ||
-					minMinor > 255
-				) {
-					addError(
-						file,
-						`The minimum firmware version ${config.firmwareVersion.min} is invalid. Each version part must be between 0 and 255.`,
-					);
-				}
+			}
 
-				const [maxMajor, maxMinor] = config.firmwareVersion.max
-					.split(".", 2)
-					.map((v) => parseInt(v, 10));
-				if (
-					maxMajor < 0 ||
-					maxMajor > 255 ||
-					maxMinor < 0 ||
-					maxMinor > 255
-				) {
-					addError(
-						file,
-						`The maximum firmware version ${config.firmwareVersion.max} is invalid. Each version part must be between 0 and 255.`,
-					);
-				}
+			// Check if one of the firmware versions has a leading zero
+			const leadingZeroMajor = /^0\d+\./;
+			const leadingZeroMinor = /\.0\d+$/;
+			if (
+				leadingZeroMajor.test(config.firmwareVersion.min) ||
+				leadingZeroMinor.test(config.firmwareVersion.min)
+			) {
+				addError(
+					file,
+					`The minimum firmware version ${config.firmwareVersion.min} is invalid. Leading zeroes are not permitted.`,
+				);
+			}
+			if (
+				leadingZeroMajor.test(config.firmwareVersion.max) ||
+				leadingZeroMinor.test(config.firmwareVersion.max)
+			) {
+				addError(
+					file,
+					`The maximum firmware version ${config.firmwareVersion.max} is invalid. Leading zeroes are not permitted.`,
+				);
 			}
 		}
 	}

diff --git a/packages/config/src/Devices.ts b/packages/config/src/Devices.ts
@@ -213,9 +213,7 @@ devices is malformed (not an object or type/id that is not a 4-digit hex key)`,
 			({ productType, productId }) => ({ productType, productId }),
 		);
 
-		if (definition.firmwareVersion === false) {
-			this.firmwareVersion = false;
-		} else if (
+		if (
 			!isObject(definition.firmwareVersion) ||
 			!isFirmwareVersion(definition.firmwareVersion.min) ||
 			!isFirmwareVersion(definition.firmwareVersion.max)
@@ -327,7 +325,7 @@ compat is not an object`,
 		productType: string;
 		productId: string;
 	}[];
-	public readonly firmwareVersion: FirmwareVersionRange | false;
+	public readonly firmwareVersion: FirmwareVersionRange;
 	public readonly associations?: ReadonlyMap<number, AssociationConfig>;
 	public readonly paramInformation?: ParamInfoMap;
 	/**