Skip to content

Commit

Permalink
Remove legacy_availability_payload, legacy_api. Remove legacy fro…
Browse files Browse the repository at this point in the history
…m configure.
  • Loading branch information
Nerivec committed Oct 7, 2024
1 parent 0c29f97 commit 087fe3e
Show file tree
Hide file tree
Showing 14 changed files with 82 additions and 150 deletions.
12 changes: 1 addition & 11 deletions lib/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,8 @@ export class Controller {
}

// Start zigbee
let startResult;
try {
startResult = await this.zigbee.start();
await this.zigbee.start();
this.eventBus.onAdapterDisconnected(this, this.onZigbeeAdapterDisconnected);
} catch (error) {
logger.error('Failed to start zigbee');
Expand All @@ -149,15 +148,6 @@ export class Controller {
return await this.exit(1);
}

// Disable some legacy options on new network creation
if (startResult === 'reset') {
settings.set(['advanced', 'homeassistant_legacy_entity_attributes'], false);
settings.set(['advanced', 'legacy_api'], false);
settings.set(['advanced', 'legacy_availability_payload'], false);
settings.set(['device_options', 'legacy'], false);
await this.enableDisableExtension(false, 'BridgeLegacy');
}

// Log zigbee clients on startup
let deviceCount = 0;

Expand Down
2 changes: 1 addition & 1 deletion lib/extension/availability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ export default class Availability extends Extension {
}

const topic = `${entity.name}/availability`;
const payload = utils.availabilityPayload(available ? 'online' : 'offline', settings.get());
const payload = JSON.stringify({state: available ? 'online' : 'offline'});
this.availabilityCache[entity.ID] = available;
await this.mqtt.publish(topic, payload, {retain: true, qos: 1});

Expand Down
16 changes: 1 addition & 15 deletions lib/extension/configure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export default class Configure extends Extension {
private configuring = new Set();
private attempts: {[s: string]: number} = {};
private topic = `${settings.get().mqtt.base_topic}/bridge/request/device/configure`;
private legacyTopic = `${settings.get().mqtt.base_topic}/bridge/configure`;

@bind private async onReconfigure(data: eventdata.Reconfigure): Promise<void> {
// Disabling reporting unbinds some cluster which could be bound by configure, re-setup.
Expand All @@ -29,20 +28,7 @@ export default class Configure extends Extension {
}

@bind private async onMQTTMessage(data: eventdata.MQTTMessage): Promise<void> {
if (data.topic === this.legacyTopic) {
const device = this.zigbee.resolveEntity(data.message);
if (!device || !(device instanceof Device)) {
logger.error(`Device '${data.message}' does not exist`);
return;
}

if (!device.definition || !device.definition.configure) {
logger.warning(`Skipping configure of '${device.name}', device does not require this.`);
return;
}

await this.configure(device, 'mqtt_message', true);
} else if (data.topic === this.topic) {
if (data.topic === this.topic) {
const message = utils.parseJSON(data.message, data.message);
const ID = typeof message === 'object' && message.id !== undefined ? message.id : message;
let error: string | undefined;
Expand Down
5 changes: 2 additions & 3 deletions lib/extension/homeassistant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,7 @@ export default class HomeAssistant extends Extension {
if (isDevice && entity.options.disabled) {
// Mark disabled device always as unavailable
payload.availability.forEach((a: KeyValue) => (a.value_template = '{{ "offline" }}'));
} else if (!settings.get().advanced.legacy_availability_payload) {
} else {
payload.availability.forEach((a: KeyValue) => (a.value_template = '{{ value_json.state }}'));
}
} else {
Expand Down Expand Up @@ -2017,7 +2017,6 @@ export default class HomeAssistant extends Extension {
const discovery: DiscoveryEntry[] = [];
const bridge = new Bridge(coordinatorIeeeAddress, coordinatorVersion, discovery);
const baseTopic = `${settings.get().mqtt.base_topic}/${bridge.name}`;
const legacyAvailability = settings.get().advanced.legacy_availability_payload;

discovery.push(
// Binary sensors.
Expand All @@ -2031,7 +2030,7 @@ export default class HomeAssistant extends Extension {
entity_category: 'diagnostic',
state_topic: true,
state_topic_postfix: 'state',
value_template: !legacyAvailability ? '{{ value_json.state }}' : '{{ value }}',
value_template: '{{ value_json.state }}',
payload_on: 'online',
payload_off: 'offline',
availability: false,
Expand Down
6 changes: 3 additions & 3 deletions lib/mqtt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default class MQTT {
const options: mqtt.IClientOptions = {
will: {
topic: `${settings.get().mqtt.base_topic}/bridge/state`,
payload: Buffer.from(utils.availabilityPayload('offline', settings.get())),
payload: Buffer.from(JSON.stringify({state: 'offline'})),
retain: settings.get().mqtt.force_disable_retain ? false : true,
qos: 1,
},
Expand Down Expand Up @@ -123,13 +123,13 @@ export default class MQTT {
}

@bind async publishStateOnline(): Promise<void> {
await this.publish('bridge/state', utils.availabilityPayload('online', settings.get()), {retain: true, qos: 0});
await this.publish('bridge/state', JSON.stringify({state: 'online'}), {retain: true, qos: 0});
}

async disconnect(): Promise<void> {
clearTimeout(this.connectionTimer);
clearTimeout(this.republishRetainedTimer);
await this.publish('bridge/state', utils.availabilityPayload('offline', settings.get()), {retain: true, qos: 0});
await this.publish('bridge/state', JSON.stringify({state: 'offline'}), {retain: true, qos: 0});
this.eventBus.removeListeners(this);
logger.info('Disconnecting from MQTT server');
this.client?.end();
Expand Down
2 changes: 0 additions & 2 deletions lib/types/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,6 @@ declare global {
groups: {[s: string]: OptionalProps<Omit<GroupOptions, 'ID'>, 'devices'>};
device_options: KeyValue;
advanced: {
legacy_api: boolean;
legacy_availability_payload: boolean;
log_rotation: boolean;
log_symlink_current: boolean;
log_output: ('console' | 'file' | 'syslog')[];
Expand Down
2 changes: 0 additions & 2 deletions lib/util/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ const defaults: RecursivePartial<Settings> = {
},
device_options: {},
advanced: {
legacy_api: true,
legacy_availability_payload: true,
log_rotation: true,
log_symlink_current: false,
log_output: ['console', 'file'],
Expand Down
5 changes: 0 additions & 5 deletions lib/util/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,10 +329,6 @@ function isZHGroup(obj: unknown): obj is zh.Group {
return obj?.constructor.name.toLowerCase() === 'group';
}

function availabilityPayload(state: 'online' | 'offline', settings: Settings): string {
return settings.advanced.legacy_availability_payload ? state : JSON.stringify({state});
}

const hours = (hours: number): number => 1000 * 60 * 60 * hours;
const minutes = (minutes: number): number => 1000 * 60 * minutes;
const seconds = (seconds: number): number => 1000 * seconds;
Expand Down Expand Up @@ -447,7 +443,6 @@ export default {
sanitizeImageParameter,
isAvailabilityEnabledForEntity,
publishLastSeen,
availabilityPayload,
getAllFiles,
filterProperties,
flatten,
Expand Down
29 changes: 14 additions & 15 deletions test/availability.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ describe('Availability', () => {
});

it('Should publish availability on startup for device where it is enabled for', async () => {
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_color/availability', 'online', {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/remote/availability', 'online', {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_color/availability', stringify({state: 'online'}), {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/remote/availability', stringify({state: 'online'}), {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).not.toHaveBeenCalledWith(
'zigbee2mqtt/bulb_color_2/availability',
'online',
stringify({state: 'online'}),
{retain: true, qos: 1},
expect.any(Function),
);
Expand Down Expand Up @@ -109,7 +109,7 @@ describe('Availability', () => {
await setTimeAndAdvanceTimers(utils.minutes(7));
expect(devices.bulb_color.ping).toHaveBeenCalledTimes(1);
expect(devices.bulb_color.ping).toHaveBeenNthCalledWith(1, true);
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_color/availability', 'offline', {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_color/availability', stringify({state: 'offline'}), {retain: true, qos: 1}, expect.any(Function));
});

it('Shouldnt do anything for a device when availability: false is set for device', async () => {
Expand All @@ -123,7 +123,7 @@ describe('Availability', () => {
MQTT.publish.mockClear();
await setTimeAndAdvanceTimers(utils.hours(26));
expect(devices.remote.ping).toHaveBeenCalledTimes(0);
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/remote/availability', 'offline', {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/remote/availability', stringify({state: 'offline'}), {retain: true, qos: 1}, expect.any(Function));
});

it('Should reset ping timer when device last seen changes for active device', async () => {
Expand All @@ -133,7 +133,7 @@ describe('Availability', () => {
expect(devices.bulb_color.ping).toHaveBeenCalledTimes(0);

await zigbeeHerdsman.events.lastSeenChanged({device: devices.bulb_color});
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_color/availability', 'offline', {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_color/availability', stringify({state: 'offline'}), {retain: true, qos: 1}, expect.any(Function));

await setTimeAndAdvanceTimers(utils.minutes(7));
expect(devices.bulb_color.ping).toHaveBeenCalledTimes(0);
Expand Down Expand Up @@ -165,7 +165,7 @@ describe('Availability', () => {
expect(devices.remote.ping).toHaveBeenCalledTimes(0);

await zigbeeHerdsman.events.lastSeenChanged({device: devices.remote});
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/remote/availability', 'offline', {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/remote/availability', stringify({state: 'offline'}), {retain: true, qos: 1}, expect.any(Function));

await setTimeAndAdvanceTimers(utils.hours(25));
expect(devices.remote.ping).toHaveBeenCalledTimes(0);
Expand All @@ -178,12 +178,12 @@ describe('Availability', () => {
MQTT.publish.mockClear();

await setTimeAndAdvanceTimers(utils.minutes(15));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_color/availability', 'offline', {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_color/availability', stringify({state: 'offline'}), {retain: true, qos: 1}, expect.any(Function));

devices.bulb_color.lastSeen = Date.now();
await zigbeeHerdsman.events.lastSeenChanged({device: devices.bulb_color});
await flushPromises();
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_color/availability', 'online', {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_color/availability', stringify({state: 'online'}), {retain: true, qos: 1}, expect.any(Function));
});

it('Should allow to change availability timeout via device options', async () => {
Expand Down Expand Up @@ -299,13 +299,12 @@ describe('Availability', () => {
await flushPromises();

expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_color/availability', '', {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_new_name/availability', 'online', {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_new_name/availability', stringify({state: 'online'}), {retain: true, qos: 1}, expect.any(Function));
await setTimeAndAdvanceTimers(utils.hours(12));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_new_name/availability', 'offline', {retain: true, qos: 1}, expect.any(Function));
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bulb_new_name/availability', stringify({state: 'offline'}), {retain: true, qos: 1}, expect.any(Function));
});

it('Should publish availability payload in JSON format', async () => {
settings.set(['advanced', 'legacy_availability_payload'], false);
await resetExtension();
devices.remote.ping.mockClear();
MQTT.publish.mockClear();
Expand All @@ -326,15 +325,15 @@ describe('Availability', () => {

expect(MQTT.publish).toHaveBeenCalledWith(
'zigbee2mqtt/group_tradfri_remote/availability',
'online',
stringify({state: 'online'}),
{retain: true, qos: 1},
expect.any(Function),
);
MQTT.publish.mockClear();
await setTimeAndAdvanceTimers(utils.minutes(12));
expect(MQTT.publish).toHaveBeenCalledWith(
'zigbee2mqtt/group_tradfri_remote/availability',
'offline',
stringify({state: 'offline'}),
{retain: true, qos: 1},
expect.any(Function),
);
Expand All @@ -344,7 +343,7 @@ describe('Availability', () => {
await flushPromises();
expect(MQTT.publish).toHaveBeenCalledWith(
'zigbee2mqtt/group_tradfri_remote/availability',
'online',
stringify({state: 'online'}),
{retain: true, qos: 1},
expect.any(Function),
);
Expand Down
4 changes: 0 additions & 4 deletions test/bridge.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ describe('Bridge', () => {
beforeAll(async () => {
jest.useFakeTimers();
mockRestart = jest.fn();
settings.set(['advanced', 'legacy_api'], false);
controller = new Controller(mockRestart, jest.fn());
await controller.start();
await flushPromises();
Expand All @@ -60,7 +59,6 @@ describe('Bridge', () => {
MQTT.mock.reconnecting = false;
data.writeDefaultConfiguration();
settings.reRead();
settings.set(['advanced', 'legacy_api'], false);
data.writeDefaultState();
logger.info.mockClear();
logger.warning.mockClear();
Expand Down Expand Up @@ -101,8 +99,6 @@ describe('Bridge', () => {
elapsed: false,
ext_pan_id: [221, 221, 221, 221, 221, 221, 221, 221],
last_seen: 'disable',
legacy_api: false,
legacy_availability_payload: true,
log_debug_namespace_ignore: '',
log_debug_to_mqtt_frontend: false,
log_directory: directory,
Expand Down
20 changes: 0 additions & 20 deletions test/configure.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,26 +191,6 @@ describe('Configure', () => {
);
});

it('Legacy api: Should allow to reconfigure manually', async () => {
mockClear(zigbeeHerdsman.devices.remote);
expectRemoteNotConfigured();
await MQTT.events.message('zigbee2mqtt/bridge/configure', 'remote');
await flushPromises();
expectRemoteConfigured();
});

it('Legacy api: Shouldnt manually reconfigure when device does not exist', async () => {
await MQTT.events.message('zigbee2mqtt/bridge/configure', 'remote_random_non_existing');
await flushPromises();
expect(logger.error).toHaveBeenCalledWith(`Device 'remote_random_non_existing' does not exist`);
});

it('Legacy api: Should skip reconfigure when device does not require this', async () => {
await MQTT.events.message('zigbee2mqtt/bridge/configure', '0x0017882104a44559');
await flushPromises();
expect(logger.warning).toHaveBeenCalledWith(`Skipping configure of 'TS0601_thermostat', device does not require this.`);
});

it('Should not configure when interview not completed', async () => {
const device = zigbeeHerdsman.devices.remote;
delete device.meta.configured;
Expand Down
17 changes: 3 additions & 14 deletions test/controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('Controller', () => {
expect(logger.info).toHaveBeenCalledWith('0x0017880104e45518 (0x0017880104e45518): Not supported (EndDevice)');
expect(MQTT.connect).toHaveBeenCalledTimes(1);
expect(MQTT.connect).toHaveBeenCalledWith('mqtt://localhost', {
will: {payload: Buffer.from('offline'), retain: true, topic: 'zigbee2mqtt/bridge/state', qos: 1},
will: {payload: Buffer.from('{"state":"offline"}'), retain: true, topic: 'zigbee2mqtt/bridge/state', qos: 1},
});
expect(MQTT.publish).toHaveBeenCalledWith(
'zigbee2mqtt/bulb',
Expand Down Expand Up @@ -140,7 +140,7 @@ describe('Controller', () => {
await flushPromises();
expect(MQTT.connect).toHaveBeenCalledTimes(1);
const expected = {
will: {payload: Buffer.from('offline'), retain: true, topic: 'zigbee2mqtt/bridge/state', qos: 1},
will: {payload: Buffer.from('{"state":"offline"}'), retain: true, topic: 'zigbee2mqtt/bridge/state', qos: 1},
keepalive: 30,
ca: Buffer.from([99, 97]),
key: Buffer.from([107, 101, 121]),
Expand Down Expand Up @@ -895,7 +895,7 @@ describe('Controller', () => {
await flushPromises();
expect(MQTT.connect).toHaveBeenCalledTimes(1);
const expected = {
will: {payload: Buffer.from('offline'), retain: false, topic: 'zigbee2mqtt/bridge/state', qos: 1},
will: {payload: Buffer.from('{"state":"offline"}'), retain: false, topic: 'zigbee2mqtt/bridge/state', qos: 1},
};
expect(MQTT.connect).toHaveBeenCalledWith('mqtt://localhost', expected);
});
Expand Down Expand Up @@ -930,17 +930,6 @@ describe('Controller', () => {
expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/fo', 'bar', {retain: false, qos: 0}, expect.any(Function));
});

it('Should disable legacy options on new network start', async () => {
settings.set(['homeassistant'], true);
settings.reRead();
expect(settings.get().homeassistant.legacy_entity_attributes).toBeTruthy();
expect(settings.get().advanced.legacy_api).toBeTruthy();
zigbeeHerdsman.start.mockReturnValueOnce('reset');
await controller.start();
expect(settings.get().homeassistant.legacy_entity_attributes).toBeFalsy();
expect(settings.get().advanced.legacy_api).toBeFalsy();
});

it('Should publish last seen changes', async () => {
settings.set(['advanced', 'last_seen'], 'epoch');
await controller.start();
Expand Down
2 changes: 1 addition & 1 deletion test/frontend.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ describe('Frontend', () => {
const allTopics = mockWSClient.implementation.send.mock.calls.map((m) => JSON.parse(m).topic);
expect(allTopics).toContain('bridge/devices');
expect(allTopics).toContain('bridge/info');
expect(mockWSClient.implementation.send).toHaveBeenCalledWith(stringify({topic: 'bridge/state', payload: 'online'}));
expect(mockWSClient.implementation.send).toHaveBeenCalledWith(stringify({topic: 'bridge/state', payload: {state: 'online'}}));
expect(mockWSClient.implementation.send).toHaveBeenCalledWith(stringify({topic: 'remote', payload: {brightness: 255}}));

// Message
Expand Down
Loading

0 comments on commit 087fe3e

Please sign in to comment.