fix: Various fixes to align behaviors (+add benchmarks for dev) (#28011)

Author: Nerivec

.github/workflows/ci.yml

@@ -3,9 +3,10 @@ name: CI
 on:
   push:
     tags:        
-      - '**' 
-    branches-ignore:
-      - dev-types
+      - '[0-9]+.[0-9]+.[0-9]+' 
+    branches:
+      - dev
+      - master
   pull_request:
 
 permissions:
@@ -47,6 +48,13 @@ jobs:
             - name: Test
               run: pnpm run test:coverage
 
+            - name: Bench
+              if: github.ref == 'refs/heads/dev' || github.event_name == 'pull_request'
+              uses: CodSpeedHQ/action@v3
+              with:
+                run: pnpm run bench
+                token: ${{ secrets.CODSPEED_TOKEN }}
+
             - name: Log in to the Docker container registry
               if: (github.ref == 'refs/heads/dev' || startsWith(github.ref, 'refs/tags/')) && github.event_name == 'push'
               uses: docker/login-action@v3

.gitignore

@@ -77,6 +77,7 @@ tsconfig.types.tsbuildinfo
 # Ignore config
 data/*
 !data/configuration.example.yaml
+data-bench
 
 # commit-user-lookup.json
 scripts/commit-user-lookup.json

lib/controller.ts

@@ -28,10 +28,10 @@ import utils from "./util/utils";
 import Zigbee from "./zigbee";
 
 export class Controller {
-    private eventBus: EventBus;
-    private zigbee: Zigbee;
-    private state: State;
-    private mqtt: Mqtt;
+    public readonly eventBus: EventBus;
+    public readonly zigbee: Zigbee;
+    public readonly state: State;
+    public readonly mqtt: Mqtt;
     private restartCallback: () => Promise<void>;
     private exitCallback: (code: number, restart: boolean) => Promise<void>;
     public readonly extensions: Set<Extension>;

lib/extension/bind.ts

@@ -9,10 +9,9 @@ import Group from "../model/group";
 import type {Zigbee2MQTTAPI, Zigbee2MQTTResponseEndpoints} from "../types/api";
 import logger from "../util/logger";
 import * as settings from "../util/settings";
-import utils from "../util/utils";
+import utils, {DEFAULT_BIND_GROUP_ID} from "../util/utils";
 import Extension from "./extension";
 
-const TOPIC_REGEX = new RegExp(`^${settings.get().mqtt.base_topic}/bridge/request/device/(bind|unbind)`);
 const ALL_CLUSTER_CANDIDATES: readonly ClusterName[] = [
     "genScenes",
     "genOnOff",
@@ -28,7 +27,7 @@ const ALL_CLUSTER_CANDIDATES: readonly ClusterName[] = [
 ];
 
 // See zigbee-herdsman-converters
-const DEFAULT_BIND_GROUP = {type: "group_number", ID: 901, name: "default_bind_group"};
+const DEFAULT_BIND_GROUP = {type: "group_number", ID: DEFAULT_BIND_GROUP_ID, name: "default_bind_group"};
 const DEFAULT_REPORT_CONFIG = {minimumReportInterval: 5, maximumReportInterval: 3600, reportableChange: 1};
 
 const getColorCapabilities = async (endpoint: zh.Endpoint): Promise<{colorTemperature: boolean; colorXY: boolean}> => {
@@ -204,6 +203,7 @@ interface ParsedMQTTMessage {
 }
 
 export default class Bind extends Extension {
+    #topicRegex = new RegExp(`^${settings.get().mqtt.base_topic}/bridge/request/device/(bind|unbind)`);
     private pollDebouncers: {[s: string]: () => void} = {};
 
     // biome-ignore lint/suspicious/useAwait: API
@@ -216,7 +216,7 @@ export default class Bind extends Extension {
     private parseMQTTMessage(
         data: eventdata.MQTTMessage,
     ): [raw: KeyValue | undefined, parsed: ParsedMQTTMessage | undefined, error: string | undefined] {
-        if (data.topic.match(TOPIC_REGEX)) {
+        if (data.topic.match(this.#topicRegex)) {
             const type = data.topic.endsWith("unbind") ? "unbind" : "bind";
             let skipDisableReporting = false;
             const message = JSON.parse(data.message) as Zigbee2MQTTAPI["bridge/request/device/bind"];

lib/extension/bridge.ts

@@ -14,12 +14,11 @@ import type {Zigbee2MQTTAPI, Zigbee2MQTTDevice, Zigbee2MQTTResponse, Zigbee2MQTT
 import data from "../util/data";
 import logger from "../util/logger";
 import * as settings from "../util/settings";
-import utils, {assertString} from "../util/utils";
+import utils, {assertString, DEFAULT_BIND_GROUP_ID} from "../util/utils";
 import Extension from "./extension";
 
-const REQUEST_REGEX = new RegExp(`${settings.get().mqtt.base_topic}/bridge/request/(.*)`);
-
 export default class Bridge extends Extension {
+    #requestRegex = new RegExp(`${settings.get().mqtt.base_topic}/bridge/request/(.*)`);
     // set on `start`
     #osInfo!: Zigbee2MQTTAPI["bridge/info"]["os"];
     private zigbee2mqttVersion!: {commitHash?: string; version: string};
@@ -197,7 +196,7 @@ export default class Bridge extends Extension {
     }
 
     @bind async onMQTTMessage(data: eventdata.MQTTMessage): Promise<void> {
-        const match = data.topic.match(REQUEST_REGEX);
+        const match = data.topic.match(this.#requestRegex);
 
         if (!match) {
             return;
@@ -797,7 +796,7 @@ export default class Bridge extends Extension {
 
             groups.push({
                 id: group.ID,
-                friendly_name: group.ID === 901 ? "default_bind_group" : group.name,
+                friendly_name: group.ID === DEFAULT_BIND_GROUP_ID ? "default_bind_group" : group.name,
                 description: group.options.description,
                 scenes: utils.getScenes(group.zh),
                 members,

lib/extension/groups.ts

@@ -11,8 +11,6 @@ import * as settings from "../util/settings";
 import utils, {isLightExpose} from "../util/utils";
 import Extension from "./extension";
 
-const TOPIC_REGEX = new RegExp(`^${settings.get().mqtt.base_topic}/bridge/request/group/members/(remove|add|remove_all)$`);
-
 const STATE_PROPERTIES: Readonly<Record<string, (value: string, exposes: zhc.Expose[]) => boolean>> = {
     state: () => true,
     brightness: (_value, exposes) => exposes.some((e) => isLightExpose(e) && e.features.some((f) => f.name === "brightness")),
@@ -38,6 +36,7 @@ interface ParsedMQTTMessage {
 }
 
 export default class Groups extends Extension {
+    #topicRegex = new RegExp(`^${settings.get().mqtt.base_topic}/bridge/request/group/members/(remove|add|remove_all)$`);
     private lastOptimisticState: {[s: string]: KeyValue} = {};
 
     // biome-ignore lint/suspicious/useAwait: API
@@ -185,7 +184,7 @@ export default class Groups extends Extension {
     private parseMQTTMessage(
         data: eventdata.MQTTMessage,
     ): [raw: KeyValue | undefined, parsed: ParsedMQTTMessage | undefined, error: string | undefined] {
-        const topicRegexMatch = data.topic.match(TOPIC_REGEX);
+        const topicRegexMatch = data.topic.match(this.#topicRegex);
 
         if (topicRegexMatch) {
             const type = topicRegexMatch[1] as "remove" | "add" | "remove_all";
@@ -273,23 +272,23 @@ export default class Groups extends Extension {
         try {
             if (type === "add") {
                 assert(resolvedGroup, "`resolvedGroup` is missing");
-                logger.info(`Adding '${resolvedDevice.name}' to '${resolvedGroup.name}'`);
+                logger.info(`Adding endpoint '${resolvedEndpoint.ID}' of device '${resolvedDevice.name}' to group '${resolvedGroup.name}'`);
                 await resolvedEndpoint.addToGroup(resolvedGroup.zh);
                 changedGroups.push(resolvedGroup);
                 // biome-ignore lint/style/noNonNullAssertion: valid from resolved asserts
                 const respPayload = {device: deviceKey!, endpoint: endpointKey!, group: groupKey!};
                 await this.publishResponse<"bridge/response/group/members/add">(parsed.type, raw, respPayload);
             } else if (type === "remove") {
                 assert(resolvedGroup, "`resolvedGroup` is missing");
-                logger.info(`Removing '${resolvedDevice.name}' from '${resolvedGroup.name}'`);
+                logger.info(`Removing endpoint '${resolvedEndpoint.ID}' of device '${resolvedDevice.name}' from group '${resolvedGroup.name}'`);
                 await resolvedEndpoint.removeFromGroup(resolvedGroup.zh);
                 changedGroups.push(resolvedGroup);
                 // biome-ignore lint/style/noNonNullAssertion: valid from resolved asserts
                 const respPayload = {device: deviceKey!, endpoint: endpointKey!, group: groupKey!};
                 await this.publishResponse<"bridge/response/group/members/remove">(parsed.type, raw, respPayload);
             } else {
                 // remove_all
-                logger.info(`Removing '${resolvedDevice.name}' from all groups`);
+                logger.info(`Removing endpoint '${resolvedEndpoint.ID}' of device '${resolvedDevice.name}' from all groups`);
 
                 for (const group of this.zigbee.groupsIterator((g) => g.members.includes(resolvedEndpoint))) {
                     changedGroups.push(group);
@@ -301,7 +300,7 @@ export default class Groups extends Extension {
                 await this.publishResponse<"bridge/response/group/members/remove_all">(parsed.type, raw, respPayload);
             }
         } catch (e) {
-            const errorMsg = `Failed to ${type} from group (${(e as Error).message})`;
+            const errorMsg = `Failed to ${type} ${type === "add" ? "to" : "from"} group (${(e as Error).message})`;
             await this.publishResponse(parsed.type, raw, {}, errorMsg);
             // biome-ignore lint/style/noNonNullAssertion: always Error
             logger.debug((e as Error).stack!);

lib/extension/networkMap.ts

@@ -13,15 +13,15 @@ const SUPPORTED_FORMATS = ["raw", "graphviz", "plantuml"];
  * This extension creates a network map
  */
 export default class NetworkMap extends Extension {
-    private topic = `${settings.get().mqtt.base_topic}/bridge/request/networkmap`;
+    #topic = `${settings.get().mqtt.base_topic}/bridge/request/networkmap`;
 
     // biome-ignore lint/suspicious/useAwait: API
     override async start(): Promise<void> {
         this.eventBus.onMQTTMessage(this, this.onMQTTMessage);
     }
 
     @bind async onMQTTMessage(data: eventdata.MQTTMessage): Promise<void> {
-        if (data.topic === this.topic) {
+        if (data.topic === this.#topic) {
             const message = utils.parseJSON(data.message, data.message) as Zigbee2MQTTAPI["bridge/request/networkmap"];
 
             try {

lib/extension/otaUpdate.ts

@@ -25,12 +25,11 @@ export interface UpdatePayload {
     };
 }
 
-const topicRegex = new RegExp(
-    `^${settings.get().mqtt.base_topic}/bridge/request/device/ota_update/(update|check|schedule|unschedule)/?(downgrade)?`,
-    "i",
-);
-
 export default class OTAUpdate extends Extension {
+    #topicRegex = new RegExp(
+        `^${settings.get().mqtt.base_topic}/bridge/request/device/ota_update/(update|check|schedule|unschedule)/?(downgrade)?`,
+        "i",
+    );
     private inProgress = new Set<string>();
     private lastChecked = new Map<string, number>();
     private scheduledUpgrades = new Set<string>();
@@ -252,7 +251,7 @@ export default class OTAUpdate extends Extension {
     }
 
     @bind async onMQTTMessage(data: eventdata.MQTTMessage): Promise<void> {
-        const topicMatch = data.topic.match(topicRegex);
+        const topicMatch = data.topic.match(this.#topicRegex);
 
         if (!topicMatch) {
             return;

lib/extension/publish.ts

@@ -9,12 +9,12 @@ import * as settings from "../util/settings";
 import utils from "../util/utils";
 import Extension from "./extension";
 
+// TODO: get rid of this, use class member
 let topicGetSetRegex: RegExp;
-// Used by `publish.test.js` to reload regex when changing `mqtt.base_topic`.
+// Used by `publish.test.ts` to reload regex when changing `mqtt.base_topic`.
 export const loadTopicGetSetRegex = (): void => {
     topicGetSetRegex = new RegExp(`^${settings.get().mqtt.base_topic}/(?!bridge)(.+?)/(get|set)(?:/(.+))?$`);
 };
-loadTopicGetSetRegex();
 
 const STATE_VALUES: ReadonlyArray<string> = ["on", "off", "toggle", "open", "close", "stop", "lock", "unlock"];
 const SCENE_CONVERTER_KEYS: ReadonlyArray<string> = ["scene_store", "scene_add", "scene_remove", "scene_remove_all", "scene_rename"];
@@ -29,6 +29,7 @@ interface ParsedTopic {
 export default class Publish extends Extension {
     // biome-ignore lint/suspicious/useAwait: API
     override async start(): Promise<void> {
+        loadTopicGetSetRegex();
         this.eventBus.onMQTTMessage(this, this.onMQTTMessage);
     }
 

lib/model/group.ts

@@ -1,6 +1,6 @@
 import type * as zhc from "zigbee-herdsman-converters";
-
 import * as settings from "../util/settings";
+import {DEFAULT_BIND_GROUP_ID} from "../util/utils";
 
 export default class Group {
     public zh: zh.Group;
@@ -23,6 +23,12 @@ export default class Group {
         this.resolveDevice = resolveDevice;
     }
 
+    ensureInSettings(): void {
+        if (this.ID !== DEFAULT_BIND_GROUP_ID && !settings.getGroup(this.ID)) {
+            settings.addGroup(this.name, this.ID.toString());
+        }
+    }
+
     hasMember(device: Device): boolean {
         return !!device.zh.endpoints.find((e) => this.zh.members.includes(e));
     }