Skip to content

Commit

Permalink
fix: Permit join optimisation (#1251)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Nerivec
Nerivec authored Dec 8, 2024
1 parent c7aa454 commit 5dca3bc
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 68 deletions.
52 changes: 21 additions & 31 deletions src/controller/controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ class Controller extends events.EventEmitter<ControllerEventMap> {
// @ts-expect-error assigned and validated in start()
private touchlink: Touchlink;

private permitJoinTimeoutTimer: NodeJS.Timeout | undefined;
private permitJoinTimeout: number;
private permitJoinTimer: NodeJS.Timeout | undefined;
private permitJoinEnd?: number;
private backupTimer: NodeJS.Timeout | undefined;
private databaseSaveTimer: NodeJS.Timeout | undefined;
private stopping: boolean;
Expand All @@ -99,7 +99,6 @@ class Controller extends events.EventEmitter<ControllerEventMap> {
this.adapterDisconnected = true; // set false after adapter.start() is successfully called
this.options = mixinDeep(JSON.parse(JSON.stringify(DefaultOptions)), options);
this.unknownDevices = new Set();
this.permitJoinTimeout = 0;

// Validate options
for (const channel of this.options.network.channelList) {
Expand Down Expand Up @@ -284,9 +283,9 @@ class Controller extends events.EventEmitter<ControllerEventMap> {
}

public async permitJoin(time: number, device?: Device): Promise<void> {
clearInterval(this.permitJoinTimeoutTimer);
this.permitJoinTimeoutTimer = undefined;
this.permitJoinTimeout = 0;
clearTimeout(this.permitJoinTimer);
this.permitJoinTimer = undefined;
this.permitJoinEnd = undefined;

if (time > 0) {
// never permit more than uint8, and never permit 255 that is often equal to "forever"
Expand All @@ -295,41 +294,32 @@ class Controller extends events.EventEmitter<ControllerEventMap> {
await this.adapter.permitJoin(time, device?.networkAddress);
await this.greenPower.permitJoin(time, device?.networkAddress);

// TODO: should use setTimeout and timer only for open/close emit
// let the other end (frontend) do the sec-by-sec updating (without mqtt publish)
// Also likely creates a gap of a few secs between what Z2M says and what the stack actually has => unreliable timer end
this.permitJoinTimeout = time;
this.permitJoinTimeoutTimer = setInterval(async (): Promise<void> => {
// assumed valid number while in interval
this.permitJoinTimeout--;

if (this.permitJoinTimeout <= 0) {
clearInterval(this.permitJoinTimeoutTimer);
this.permitJoinTimeoutTimer = undefined;
this.permitJoinTimeout = 0;

this.emit('permitJoinChanged', {permitted: false, timeout: this.permitJoinTimeout});
} else {
this.emit('permitJoinChanged', {permitted: true, timeout: this.permitJoinTimeout});
}
}, 1000);
const timeMs = time * 1000;
this.permitJoinEnd = Date.now() + timeMs;
this.permitJoinTimer = setTimeout((): void => {
this.emit('permitJoinChanged', {permitted: false});

this.permitJoinTimer = undefined;
this.permitJoinEnd = undefined;
}, timeMs);

this.emit('permitJoinChanged', {permitted: true, timeout: this.permitJoinTimeout});
this.emit('permitJoinChanged', {permitted: true, time});
} else {
logger.debug('Disable joining', NS);

await this.greenPower.permitJoin(0);
await this.adapter.permitJoin(0);

this.emit('permitJoinChanged', {permitted: false, timeout: this.permitJoinTimeout});
this.emit('permitJoinChanged', {permitted: false});
}
}

/**
* @returns Timeout until permit joining expires. [0-254], with 0 being "not permitting joining".
*/
public getPermitJoinTimeout(): number {
return this.permitJoinTimeout;
public getPermitJoin(): boolean {
return this.permitJoinTimer !== undefined;
}

public getPermitJoinEnd(): number | undefined {
return this.permitJoinEnd;
}

public isStopping(): boolean {
Expand Down
2 changes: 1 addition & 1 deletion src/controller/events.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ interface DeviceLeavePayload {

interface PermitJoinChangedPayload {
permitted: boolean;
timeout?: number;
time?: number;
}

interface LastSeenChangedPayload {
Expand Down
113 changes: 77 additions & 36 deletions test/controller.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ describe('Controller', () => {
let controller: Controller;

beforeAll(async () => {
jest.useFakeTimers({doNotFake: ['setTimeout']});
jest.useFakeTimers();
Date.now = jest.fn().mockReturnValue(150);
setLogger(mockLogger);
dummyBackup = await Utils.BackupUtils.toUnifiedBackup(mockDummyBackup);
Expand Down Expand Up @@ -2343,80 +2343,111 @@ describe('Controller', () => {

it('Controller permit joining all, disabled automatically', async () => {
await controller.start();

expect(controller.getPermitJoin()).toStrictEqual(false);
expect(controller.getPermitJoinEnd()).toBeUndefined();

(Date.now as ReturnType<typeof jest.fn>).mockReturnValueOnce(0);
await controller.permitJoin(254);

expect(mockAdapterPermitJoin).toHaveBeenCalledTimes(1);
expect(mockAdapterPermitJoin.mock.calls[0][0]).toStrictEqual(254);
expect(mockAdapterPermitJoin).toHaveBeenNthCalledWith(1, 254, undefined);
expect(events.permitJoinChanged.length).toStrictEqual(1);
expect(events.permitJoinChanged[0]).toStrictEqual({permitted: true, timeout: 254});
expect(controller.getPermitJoinTimeout()).toStrictEqual(254);
expect(events.permitJoinChanged[0]).toStrictEqual({permitted: true, time: 254});
expect(controller.getPermitJoin()).toStrictEqual(true);
expect(controller.getPermitJoinEnd()).toStrictEqual(254 * 1000);

// Green power
const commisionFrameEnable = Zcl.Frame.create(1, 1, true, undefined, 2, 'commisioningMode', 33, {options: 0x0b, commisioningWindow: 254}, {});

expect(mocksendZclFrameToAll).toHaveBeenCalledTimes(1);
expect(mocksendZclFrameToAll.mock.calls[0][0]).toStrictEqual(ZSpec.GP_ENDPOINT);
expect(mocksendZclFrameToAll).toHaveBeenNthCalledWith(
1,
ZSpec.GP_ENDPOINT,
expect.any(Object),
ZSpec.GP_ENDPOINT,
ZSpec.BroadcastAddress.RX_ON_WHEN_IDLE,
);
expect(deepClone(mocksendZclFrameToAll.mock.calls[0][1])).toStrictEqual(deepClone(commisionFrameEnable));
expect(mocksendZclFrameToAll.mock.calls[0][2]).toStrictEqual(ZSpec.GP_ENDPOINT);

await jest.advanceTimersByTimeAsync(250 * 1000);

expect(mocksendZclFrameToAll).toHaveBeenCalledTimes(1);
expect(mockAdapterPermitJoin).toHaveBeenCalledTimes(1);
expect(controller.getPermitJoinTimeout()).toStrictEqual(4);
expect(controller.getPermitJoin()).toStrictEqual(true);
expect(controller.getPermitJoinEnd()).toStrictEqual(254 * 1000);

// Timer expired
await jest.advanceTimersByTimeAsync(10 * 1000);

expect(mockAdapterPermitJoin).toHaveBeenCalledTimes(1);
expect(events.permitJoinChanged.length).toStrictEqual(255);
expect(events.permitJoinChanged[254]).toStrictEqual({permitted: false, timeout: 0});
expect(controller.getPermitJoinTimeout()).toStrictEqual(0);
expect(events.permitJoinChanged.length).toStrictEqual(2);
expect(events.permitJoinChanged[1]).toStrictEqual({permitted: false});
expect(controller.getPermitJoin()).toStrictEqual(false);
expect(controller.getPermitJoinEnd()).toBeUndefined();

// Green power
expect(mocksendZclFrameToAll).toHaveBeenCalledTimes(1);
});

it('Controller permit joining all, disabled manually', async () => {
await controller.start();

expect(controller.getPermitJoin()).toStrictEqual(false);
expect(controller.getPermitJoinEnd()).toBeUndefined();

(Date.now as ReturnType<typeof jest.fn>).mockReturnValueOnce(0);
await controller.permitJoin(254);

expect(mockAdapterPermitJoin).toHaveBeenCalledTimes(1);
expect(mockAdapterPermitJoin.mock.calls[0][0]).toStrictEqual(254);
expect(mockAdapterPermitJoin).toHaveBeenNthCalledWith(1, 254, undefined);
expect(events.permitJoinChanged.length).toStrictEqual(1);
expect(events.permitJoinChanged[0]).toStrictEqual({permitted: true, timeout: 254});
expect(controller.getPermitJoinTimeout()).toStrictEqual(254);
expect(events.permitJoinChanged[0]).toStrictEqual({permitted: true, time: 254});
expect(controller.getPermitJoin()).toStrictEqual(true);
expect(controller.getPermitJoinEnd()).toStrictEqual(254 * 1000);

// Green power
const commisionFrameEnable = Zcl.Frame.create(1, 1, true, undefined, 2, 'commisioningMode', 33, {options: 0x0b, commisioningWindow: 254}, {});

expect(mocksendZclFrameToAll).toHaveBeenCalledTimes(1);
expect(mocksendZclFrameToAll.mock.calls[0][0]).toStrictEqual(ZSpec.GP_ENDPOINT);
expect(mocksendZclFrameToAll).toHaveBeenNthCalledWith(
1,
ZSpec.GP_ENDPOINT,
expect.any(Object),
ZSpec.GP_ENDPOINT,
ZSpec.BroadcastAddress.RX_ON_WHEN_IDLE,
);
expect(deepClone(mocksendZclFrameToAll.mock.calls[0][1])).toStrictEqual(deepClone(commisionFrameEnable));
expect(mocksendZclFrameToAll.mock.calls[0][2]).toStrictEqual(ZSpec.GP_ENDPOINT);

await jest.advanceTimersByTimeAsync(250 * 1000);

expect(mocksendZclFrameToAll).toHaveBeenCalledTimes(1);
expect(mockAdapterPermitJoin).toHaveBeenCalledTimes(1);
expect(events.permitJoinChanged.length).toStrictEqual(251);
expect(controller.getPermitJoinTimeout()).toStrictEqual(4);
expect(controller.getPermitJoin()).toStrictEqual(true);
expect(controller.getPermitJoinEnd()).toStrictEqual(254 * 1000);

// Disable
await controller.permitJoin(0);

expect(mockAdapterPermitJoin).toHaveBeenCalledTimes(2);
expect(mockAdapterPermitJoin.mock.calls[1][0]).toStrictEqual(0);
expect(events.permitJoinChanged.length).toStrictEqual(252);
expect(events.permitJoinChanged[251]).toStrictEqual({permitted: false, timeout: 0});
expect(controller.getPermitJoinTimeout()).toStrictEqual(0);
expect(mockAdapterPermitJoin).toHaveBeenNthCalledWith(2, 0);
expect(events.permitJoinChanged.length).toStrictEqual(2);
expect(events.permitJoinChanged[1]).toStrictEqual({permitted: false});
expect(controller.getPermitJoin()).toStrictEqual(false);
expect(controller.getPermitJoinEnd()).toBeUndefined();

// Green power
const commissionFrameDisable = Zcl.Frame.create(1, 1, true, undefined, 3, 'commisioningMode', 33, {options: 0x0a, commisioningWindow: 0}, {});

expect(mocksendZclFrameToAll).toHaveBeenCalledTimes(2);
expect(mocksendZclFrameToAll.mock.calls[1][0]).toStrictEqual(ZSpec.GP_ENDPOINT);
expect(mocksendZclFrameToAll).toHaveBeenNthCalledWith(
2,
ZSpec.GP_ENDPOINT,
expect.any(Object),
ZSpec.GP_ENDPOINT,
ZSpec.BroadcastAddress.RX_ON_WHEN_IDLE,
);
expect(deepClone(mocksendZclFrameToAll.mock.calls[1][1])).toStrictEqual(deepClone(commissionFrameDisable));
expect(mocksendZclFrameToAll.mock.calls[1][2]).toStrictEqual(ZSpec.GP_ENDPOINT);
});

it('Controller permit joining through specific device', async () => {
Expand All @@ -2425,40 +2456,50 @@ describe('Controller', () => {
await controller.permitJoin(254, controller.getDeviceByIeeeAddr('0x129'));

expect(mockAdapterPermitJoin).toHaveBeenCalledTimes(1);
expect(mockAdapterPermitJoin.mock.calls[0][0]).toStrictEqual(254);
expect(mockAdapterPermitJoin.mock.calls[0][1]).toStrictEqual(129);
expect(controller.getPermitJoinTimeout()).toStrictEqual(254);
expect(mockAdapterPermitJoin).toHaveBeenCalledWith(254, 129);
expect(events.permitJoinChanged.length).toStrictEqual(1);
expect(events.permitJoinChanged[0]).toStrictEqual({permitted: true, time: 254});
expect(controller.getPermitJoin()).toStrictEqual(true);
expect(controller.getPermitJoinEnd()).toBeGreaterThan(0);

await jest.advanceTimersByTimeAsync(120 * 1000);

expect(controller.getPermitJoin()).toStrictEqual(true);
expect(controller.getPermitJoinEnd()).toBeGreaterThan(0);

// Timer expired
await jest.advanceTimersByTimeAsync(300 * 1000);

expect(mockAdapterPermitJoin).toHaveBeenCalledTimes(1);
expect(controller.getPermitJoinTimeout()).toStrictEqual(0);
expect(events.permitJoinChanged.length).toStrictEqual(2);
expect(events.permitJoinChanged[1]).toStrictEqual({permitted: false});
expect(controller.getPermitJoin()).toStrictEqual(false);
expect(controller.getPermitJoinEnd()).toBeUndefined();
});

it('Controller permit joining for specific time', async () => {
await controller.start();
await controller.permitJoin(10);

expect(mockAdapterPermitJoin).toHaveBeenCalledTimes(1);
expect(mockAdapterPermitJoin.mock.calls[0][0]).toStrictEqual(10);
expect(mockAdapterPermitJoin).toHaveBeenCalledWith(10, undefined);
expect(events.permitJoinChanged.length).toStrictEqual(1);
expect(events.permitJoinChanged[0]).toStrictEqual({permitted: true, timeout: 10});
expect(controller.getPermitJoinTimeout()).toStrictEqual(10);
expect(events.permitJoinChanged[0]).toStrictEqual({permitted: true, time: 10});
expect(controller.getPermitJoin()).toStrictEqual(true);

await jest.advanceTimersByTimeAsync(5 * 1000);

expect(events.permitJoinChanged.length).toStrictEqual(6);
expect(events.permitJoinChanged[5]).toStrictEqual({permitted: true, timeout: 5});
expect(controller.getPermitJoinTimeout()).toStrictEqual(5);
expect(controller.getPermitJoin()).toStrictEqual(true);
expect(controller.getPermitJoinEnd()).toBeGreaterThan(0);

// Timer expired
await jest.advanceTimersByTimeAsync(7 * 1000);

expect(mockAdapterPermitJoin).toHaveBeenCalledTimes(1);
expect(events.permitJoinChanged.length).toStrictEqual(11);
expect(events.permitJoinChanged[10]).toStrictEqual({permitted: false, timeout: 0});
expect(controller.getPermitJoinTimeout()).toStrictEqual(0);
expect(events.permitJoinChanged.length).toStrictEqual(2);
expect(events.permitJoinChanged[1]).toStrictEqual({permitted: false});
expect(controller.getPermitJoin()).toStrictEqual(false);
expect(controller.getPermitJoinEnd()).toBeUndefined();
});

it('Controller permit joining for too long time throws', async () => {
Expand Down

0 comments on commit 5dca3bc

Please sign in to comment.