From bda1bac04c708ea0c35318604f926c035ff0883b Mon Sep 17 00:00:00 2001
From: Felipe Lalanne <1822826+pipex@users.noreply.github.com>
Date: Mon, 26 Feb 2024 17:49:05 -0300
Subject: [PATCH] Add support for repeated overlays

RPI firmware configuration allows repeating overlays to define
configurations on multiple devices. For instance, for configuring
multiple `ads` devices, `config.txt` needs to be setup this way

```
dtoverlay=ads1115,addr=0x48
dtoverlay=ads1115,addr=0x49
```

Before this change, the supervisor would interpret both lines as
belonging to the same overlay, preventing users from configuring multiple
devices, and leading to a loop when trying to apply configurations with
repeated overlays coming from the cloud side.

Change-type: minor
---
 src/config/backends/config-txt.ts          | 56 ++++++++++------------
 test/integration/config/config-txt.spec.ts | 49 ++++++++++++++++++-
 2 files changed, 73 insertions(+), 32 deletions(-)

diff --git a/src/config/backends/config-txt.ts b/src/config/backends/config-txt.ts
index 49c74707..d5aa0bff 100644
--- a/src/config/backends/config-txt.ts
+++ b/src/config/backends/config-txt.ts
@@ -26,9 +26,9 @@ function isArrayConfig(x: string): x is ArrayConfig {
 	return x != null && ARRAY_CONFIGS.includes(x as any);
 }
 
-// The DTOverlays type is a collection of DtParams
-type DTParam = string;
-type DTOverlays = { [name: string]: DTParam[] };
+// We use the empty string as the identifier of the base
+// overlay to allow handling the case where the user defines an
+// empty overlay as `dtoverlay=`
 const BASE_OVERLAY = '';
 
 function isBaseParam(dtparam: string): boolean {
@@ -119,8 +119,10 @@ export class ConfigTxt extends ConfigBackend {
 		const conf: ConfigTxtOptions = {};
 		const configStatements = configContents.split(/\r?\n/);
 
-		let currOverlay = BASE_OVERLAY;
-		const dtOverlays: DTOverlays = { [BASE_OVERLAY]: [] };
+		const baseParams: string[] = [];
+		const overlayQueue: Array<[string, string[]]> = [
+			[BASE_OVERLAY, baseParams],
+		];
 
 		for (const configStr of configStatements) {
 			// Don't show warnings for comments and empty lines
@@ -140,6 +142,7 @@ export class ConfigTxt extends ConfigBackend {
 				} else {
 					// dtparams and dtoverlays need to be treated as a special case
 					if (key === 'dtparam') {
+						const [, currParams] = overlayQueue[overlayQueue.length - 1];
 						// The specification allows multiple params in a line
 						const params = value.split(',');
 						params.forEach((param) => {
@@ -147,9 +150,9 @@ export class ConfigTxt extends ConfigBackend {
 								// We make sure to put the base param in the right overlays
 								// since RPI doesn't seem to be too strict about the ordering
 								// when it comes to these base params
-								dtOverlays[BASE_OVERLAY].push(value);
+								baseParams.push(value);
 							} else {
-								dtOverlays[currOverlay].push(value);
+								currParams.push(value);
 							}
 						});
 					} else if (key === 'dtoverlay') {
@@ -157,21 +160,15 @@ export class ConfigTxt extends ConfigBackend {
 						// we don't validate that the value is well formed
 						const [overlay, ...params] = value.split(',');
 
-						// Update the DTO for next dtparam
-						currOverlay = overlay;
-						if (dtOverlays[overlay] == null) {
-							dtOverlays[overlay] = [];
-						}
-
-						// Add params to the list
-						dtOverlays[overlay].push(...params);
+						// A new dtoverlay statement means we add a new entry to the
+						// queue with the given params list
+						overlayQueue.push([overlay, params]);
 					} else {
 						// Otherwise push the new value to the array
-						const arrayConf = conf[key] == null ? [] : conf[key]!;
-						arrayConf.push(value);
 						if (conf[key] == null) {
-							conf[key] = arrayConf;
+							conf[key] = [];
 						}
+						conf[key]!.push(value);
 					}
 				}
 				continue;
@@ -188,19 +185,16 @@ export class ConfigTxt extends ConfigBackend {
 			}
 		}
 
-		// Convert the base overlay to global dtparams
-		const baseOverlay = dtOverlays[BASE_OVERLAY];
-		delete dtOverlays[BASE_OVERLAY];
-		if (baseOverlay.length > 0) {
-			conf.dtparam = baseOverlay;
-		}
-
-		// Convert dtoverlays to array format
-		const overlayEntries = Object.entries(dtOverlays);
-		if (overlayEntries.length > 0) {
-			conf.dtoverlay = overlayEntries.map(([overlay, params]) =>
-				[overlay, ...params].join(','),
-			);
+		for (const [overlay, params] of overlayQueue) {
+			// Convert the base overlay to global dtparams
+			if (overlay === BASE_OVERLAY && params.length > 0) {
+				conf.dtparam = conf.dtparam != null ? conf.dtparam : [];
+				conf.dtparam.push(...params);
+			} else if (overlay !== BASE_OVERLAY) {
+				// Convert dtoverlays to array format
+				conf.dtoverlay = conf.dtoverlay != null ? conf.dtoverlay : [];
+				conf.dtoverlay.push([overlay, ...params].join(','));
+			}
 		}
 
 		return conf;
diff --git a/test/integration/config/config-txt.spec.ts b/test/integration/config/config-txt.spec.ts
index 64e17ada..8ac6814e 100644
--- a/test/integration/config/config-txt.spec.ts
+++ b/test/integration/config/config-txt.spec.ts
@@ -45,6 +45,52 @@ describe('config/config-txt', () => {
 		await tfs.restore();
 	});
 
+	it('correctly parses a config.txt file with repeated overlays', async () => {
+		const tfs = await testfs({
+			[hostUtils.pathOnBoot('config.txt')]: stripIndent`
+	        gpu_mem=64
+					avoid_warnings=1
+					dtoverlay=vc4-kms-v3d
+					dtoverlay=ads1015
+					dtparam=addr=0x48
+					dtparam=cha_cfg=4
+					dtparam=chb_cfg=5
+					dtparam=chc_cfg=6
+					dtparam=chd_cfg=7
+					dtoverlay=ads1015
+					dtparam=addr=0x49
+					dtparam=chc_enable=false
+					dtparam=chd_enable=false
+					dtparam=cha_cfg=0
+					dtparam=chb_cfg=3
+					dtparam=i2c_arm=on
+					dtparam=spi=on
+					dtparam=audio=on
+					enable_uart=0
+					gpio=8=pd
+					gpio=17=op,dh
+					`,
+		}).enable();
+
+		const configTxt = new ConfigTxt();
+
+		// Will try to parse /test/data/mnt/boot/config.txt
+		await expect(configTxt.getBootConfig()).to.eventually.deep.equal({
+			dtparam: ['i2c_arm=on', 'spi=on', 'audio=on'],
+			dtoverlay: [
+				'vc4-kms-v3d',
+				'ads1015,addr=0x48,cha_cfg=4,chb_cfg=5,chc_cfg=6,chd_cfg=7',
+				'ads1015,addr=0x49,chc_enable=false,chd_enable=false,cha_cfg=0,chb_cfg=3',
+			],
+			enable_uart: '0',
+			avoid_warnings: '1',
+			gpio: ['8=pd', '17=op,dh'],
+			gpu_mem: '64',
+		});
+
+		await tfs.restore();
+	});
+
 	it('correctly parses a config.txt file with an empty overlay', async () => {
 		const tfs = await testfs({
 			[hostUtils.pathOnBoot('config.txt')]: stripIndent`
@@ -55,6 +101,7 @@ describe('config/config-txt', () => {
 					avoid_warnings=1
 					dtoverlay=
 					dtparam=i2c=on
+					dtparam=lala=on
 					dtparam=audio=on
 					dtoverlay=ads7846
 					gpu_mem=16
@@ -66,7 +113,7 @@ describe('config/config-txt', () => {
 
 		// Will try to parse /test/data/mnt/boot/config.txt
 		await expect(configTxt.getBootConfig()).to.eventually.deep.equal({
-			dtparam: ['i2c=on', 'audio=on'],
+			dtparam: ['i2c=on', 'audio=on', 'lala=on'],
 			dtoverlay: [
 				'lirc-rpi,gpio_out_pin=17,gpio_in_pin=13,gpio_out_pin=17',
 				'ads7846',