fix: update monitor settings only if it's changed

Closes #375

Signed-off-by: Akos Kitta <[email protected]>

Author: Akos Kitta

Diff for: arduino-ide-extension/package.json

@@ -68,6 +68,7 @@
     "dateformat": "^3.0.3",
     "deepmerge": "2.0.1",
     "electron-updater": "^4.6.5",
+    "fast-json-stable-stringify": "^2.1.0",
     "fast-safe-stringify": "^2.1.1",
     "glob": "^7.1.6",
     "google-protobuf": "^3.20.1",

Diff for: arduino-ide-extension/src/common/utils.ts

@@ -1,3 +1,5 @@
+import stableJsonStringify = require('fast-json-stable-stringify');
+
 export const naturalCompare: (left: string, right: string) => number =
   require('string-natural-compare').caseInsensitive;
 
@@ -13,6 +15,19 @@ export function firstToUpperCase(what: string): string {
   return what.charAt(0).toUpperCase() + what.slice(1);
 }
 
-export function isNullOrUndefined(what: any): what is undefined | null {
+export function isNullOrUndefined(what: unknown): what is undefined | null {
   return what === undefined || what === null;
 }
+
+export function sameAs(left: unknown, right: unknown): boolean {
+  if (left === right) {
+    return true;
+  }
+  if (!left) {
+    return right === undefined;
+  }
+  if (!right) {
+    return false;
+  }
+  return stableJsonStringify(left) === stableJsonStringify(right);
+}

Diff for: arduino-ide-extension/src/node/monitor-service.ts

@@ -491,10 +491,10 @@ export class MonitorService extends CoreClientAware implements Disposable {
    */
   async changeSettings(settings: MonitorSettings): Promise<Status> {
     const config = new MonitorPortConfiguration();
-    const { pluggableMonitorSettings } = settings;
+    const { pluggableMonitorSettings = {} } = settings;
     const reconciledSettings = await this.monitorSettingsProvider.setSettings(
       this.monitorID,
-      pluggableMonitorSettings || {}
+      pluggableMonitorSettings
     );
 
     if (reconciledSettings) {
@@ -512,9 +512,13 @@ export class MonitorService extends CoreClientAware implements Disposable {
         connected: !!this.duplex,
         serialPort: this.port.address,
       },
-      pluggableMonitorSettings: reconciledSettings,
+      pluggableMonitorSettings: reconciledSettings ?? pluggableMonitorSettings,
     });
 
+    if (!reconciledSettings) {
+      return Status.OK;
+    }
+
     if (!this.duplex) {
       return Status.NOT_CONNECTED;
     }

Diff for: arduino-ide-extension/src/node/monitor-settings/monitor-settings-provider-impl.ts

@@ -17,7 +17,8 @@ import {
   longestPrefixMatch,
   reconcileSettings,
 } from './monitor-settings-utils';
-import { ILogger } from '@theia/core';
+import { deepClone, ILogger } from '@theia/core';
+import { sameAs } from '../../common/utils';
 
 const MONITOR_SETTINGS_FILE = 'pluggable-monitor-settings.json';
 
@@ -74,14 +75,17 @@ export class MonitorSettingsProviderImpl implements MonitorSettingsProvider {
   async setSettings(
     monitorId: string,
     settings: PluggableMonitorSettings
-  ): Promise<PluggableMonitorSettings> {
+  ): Promise<PluggableMonitorSettings | undefined> {
     // wait for the service to complete the init
     await this.ready.promise;
 
-    const newSettings = this.reconcileSettings(
-      settings,
-      this.monitorSettings[monitorId] || {}
-    );
+    const defaultSettings = this.monitorSettings[monitorId] || {};
+    const oldSettings = deepClone(defaultSettings);
+    const newSettings = this.reconcileSettings(settings, defaultSettings);
+    if (sameAs(oldSettings, newSettings)) {
+      // NOOP if no settings change. The `undefined` return value represents this case.
+      return undefined;
+    }
     this.monitorSettings[monitorId] = newSettings;
 
     await this.writeSettingsToFS();

Diff for: arduino-ide-extension/src/node/monitor-settings/monitor-settings-provider.ts

@@ -1,5 +1,5 @@
-import { MonitorModel } from '../../browser/monitor-model';
-import { PluggableMonitorSetting } from '../../common/protocol';
+import type { MonitorModel } from '../../browser/monitor-model';
+import type { PluggableMonitorSetting } from '../../common/protocol';
 
 export type PluggableMonitorSettings = Record<string, PluggableMonitorSetting>;
 export interface MonitorSettings {
@@ -13,8 +13,12 @@ export interface MonitorSettingsProvider {
     monitorId: string,
     defaultSettings: PluggableMonitorSettings
   ): Promise<PluggableMonitorSettings>;
+  /**
+   * The promise resolves to `undefined` if no settings change we performed. Otherwise, the reconciled settings value.
+   */
+  // TODO: find a better name for this method
   setSettings(
     monitorId: string,
     settings: PluggableMonitorSettings
-  ): Promise<PluggableMonitorSettings>;
+  ): Promise<PluggableMonitorSettings | undefined>;
 }