refactor: replace 'any' types with proper TypeScript interfaces
Major type safety improvements throughout the codebase: - Updated DEFAULT_CONFIG version to 4.2 - Replaced 'any' with proper types in systemd.ts: * displaySingleUpsStatus now uses IUpsConfig and NupstSnmp types * Fixed legacy config handling to use proper IUpsConfig format * Removed inline 'any' type annotations - Replaced 'any' with proper types in daemon.ts: * emergencyUps now properly typed as { ups: IUpsConfig, status: ISnmpUpsStatus } * Exported IUpsStatus interface for reuse * Added ISnmpUpsStatus import to disambiguate from daemon's IUpsStatus - Replaced 'any' with Record<string, unknown> in migration system: * Updated BaseMigration abstract class signatures * Updated MigrationRunner.run() signature * Updated migration-v4.0-to-v4.1.ts to use proper types * Migrations use Record<string, unknown> because they deal with unknown config schemas that are being upgraded Benefits: - TypeScript now catches type errors at compile time - Would have caught the ups.thresholds bug earlier - Better IDE autocomplete and type checking - More maintainable and self-documenting code
This commit is contained in:
14
ts/daemon.ts
14
ts/daemon.ts
@@ -4,7 +4,7 @@ import * as path from 'node:path';
|
|||||||
import { exec, execFile } from 'node:child_process';
|
import { exec, execFile } from 'node:child_process';
|
||||||
import { promisify } from 'node:util';
|
import { promisify } from 'node:util';
|
||||||
import { NupstSnmp } from './snmp/manager.ts';
|
import { NupstSnmp } from './snmp/manager.ts';
|
||||||
import type { ISnmpConfig } from './snmp/types.ts';
|
import type { ISnmpConfig, IUpsStatus as ISnmpUpsStatus } from './snmp/types.ts';
|
||||||
import { logger, type ITableColumn } from './logger.ts';
|
import { logger, type ITableColumn } from './logger.ts';
|
||||||
import { MigrationRunner } from './migrations/index.ts';
|
import { MigrationRunner } from './migrations/index.ts';
|
||||||
import { theme, symbols, getBatteryColor, getRuntimeColor, formatPowerStatus } from './colors.ts';
|
import { theme, symbols, getBatteryColor, getRuntimeColor, formatPowerStatus } from './colors.ts';
|
||||||
@@ -76,7 +76,7 @@ export interface INupstConfig {
|
|||||||
/**
|
/**
|
||||||
* UPS status tracking interface
|
* UPS status tracking interface
|
||||||
*/
|
*/
|
||||||
interface IUpsStatus {
|
export interface IUpsStatus {
|
||||||
id: string;
|
id: string;
|
||||||
name: string;
|
name: string;
|
||||||
powerStatus: 'online' | 'onBattery' | 'unknown';
|
powerStatus: 'online' | 'onBattery' | 'unknown';
|
||||||
@@ -96,7 +96,7 @@ export class NupstDaemon {
|
|||||||
|
|
||||||
/** Default configuration */
|
/** Default configuration */
|
||||||
private readonly DEFAULT_CONFIG: INupstConfig = {
|
private readonly DEFAULT_CONFIG: INupstConfig = {
|
||||||
version: '4.1',
|
version: '4.2',
|
||||||
upsDevices: [
|
upsDevices: [
|
||||||
{
|
{
|
||||||
id: 'default',
|
id: 'default',
|
||||||
@@ -171,11 +171,13 @@ export class NupstDaemon {
|
|||||||
const { config: migratedConfig, migrated } = await migrationRunner.run(parsedConfig);
|
const { config: migratedConfig, migrated } = await migrationRunner.run(parsedConfig);
|
||||||
|
|
||||||
// Save migrated config back to disk if any migrations ran
|
// Save migrated config back to disk if any migrations ran
|
||||||
|
// Cast to INupstConfig since migrations ensure the output is valid
|
||||||
|
const validConfig = migratedConfig as unknown as INupstConfig;
|
||||||
if (migrated) {
|
if (migrated) {
|
||||||
this.config = migratedConfig;
|
this.config = validConfig;
|
||||||
await this.saveConfig(this.config);
|
await this.saveConfig(this.config);
|
||||||
} else {
|
} else {
|
||||||
this.config = migratedConfig;
|
this.config = validConfig;
|
||||||
}
|
}
|
||||||
|
|
||||||
return this.config;
|
return this.config;
|
||||||
@@ -760,7 +762,7 @@ export class NupstDaemon {
|
|||||||
|
|
||||||
const rows: Array<Record<string, string>> = [];
|
const rows: Array<Record<string, string>> = [];
|
||||||
let emergencyDetected = false;
|
let emergencyDetected = false;
|
||||||
let emergencyUps: any = null;
|
let emergencyUps: { ups: IUpsConfig; status: ISnmpUpsStatus } | null = null;
|
||||||
|
|
||||||
// Check all UPS devices
|
// Check all UPS devices
|
||||||
for (const ups of this.config.upsDevices) {
|
for (const ups of this.config.upsDevices) {
|
||||||
|
@@ -28,18 +28,18 @@ export abstract class BaseMigration {
|
|||||||
/**
|
/**
|
||||||
* Check if this migration should run on the given config
|
* Check if this migration should run on the given config
|
||||||
*
|
*
|
||||||
* @param config - Raw configuration object to check
|
* @param config - Raw configuration object to check (unknown schema for migrations)
|
||||||
* @returns True if migration should run, false otherwise
|
* @returns True if migration should run, false otherwise
|
||||||
*/
|
*/
|
||||||
abstract shouldRun(config: any): Promise<boolean>;
|
abstract shouldRun(config: Record<string, unknown>): Promise<boolean>;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Perform the migration on the given config
|
* Perform the migration on the given config
|
||||||
*
|
*
|
||||||
* @param config - Raw configuration object to migrate
|
* @param config - Raw configuration object to migrate (unknown schema for migrations)
|
||||||
* @returns Migrated configuration object
|
* @returns Migrated configuration object
|
||||||
*/
|
*/
|
||||||
abstract migrate(config: any): Promise<any>;
|
abstract migrate(config: Record<string, unknown>): Promise<Record<string, unknown>>;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get human-readable name for this migration
|
* Get human-readable name for this migration
|
||||||
|
@@ -32,7 +32,9 @@ export class MigrationRunner {
|
|||||||
* @param config - Raw configuration object to migrate
|
* @param config - Raw configuration object to migrate
|
||||||
* @returns Migrated configuration and whether migrations ran
|
* @returns Migrated configuration and whether migrations ran
|
||||||
*/
|
*/
|
||||||
async run(config: any): Promise<{ config: any; migrated: boolean }> {
|
async run(
|
||||||
|
config: Record<string, unknown>,
|
||||||
|
): Promise<{ config: Record<string, unknown>; migrated: boolean }> {
|
||||||
let currentConfig = config;
|
let currentConfig = config;
|
||||||
let anyMigrationsRan = false;
|
let anyMigrationsRan = false;
|
||||||
|
|
||||||
|
@@ -49,15 +49,15 @@ export class MigrationV4_0ToV4_1 extends BaseMigration {
|
|||||||
readonly fromVersion = '4.0';
|
readonly fromVersion = '4.0';
|
||||||
readonly toVersion = '4.1';
|
readonly toVersion = '4.1';
|
||||||
|
|
||||||
async shouldRun(config: any): Promise<boolean> {
|
async shouldRun(config: Record<string, unknown>): Promise<boolean> {
|
||||||
// Run if config is version 4.0
|
// Run if config is version 4.0
|
||||||
if (config.version === '4.0') {
|
if (config.version === '4.0') {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Also run if config has upsDevices with thresholds at UPS level (v4.0 format)
|
// Also run if config has upsDevices with thresholds at UPS level (v4.0 format)
|
||||||
if (config.upsDevices && config.upsDevices.length > 0) {
|
if (Array.isArray(config.upsDevices) && config.upsDevices.length > 0) {
|
||||||
const firstDevice = config.upsDevices[0];
|
const firstDevice = config.upsDevices[0] as Record<string, unknown>;
|
||||||
// v4.0 has thresholds at UPS level, v4.1 has them in actions
|
// v4.0 has thresholds at UPS level, v4.1 has them in actions
|
||||||
return firstDevice.thresholds !== undefined;
|
return firstDevice.thresholds !== undefined;
|
||||||
}
|
}
|
||||||
@@ -65,14 +65,15 @@ export class MigrationV4_0ToV4_1 extends BaseMigration {
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
async migrate(config: any): Promise<any> {
|
async migrate(config: Record<string, unknown>): Promise<Record<string, unknown>> {
|
||||||
logger.info(`${this.getName()}: Migrating v4.0 config to v4.1 format...`);
|
logger.info(`${this.getName()}: Migrating v4.0 config to v4.1 format...`);
|
||||||
logger.dim(` - Moving thresholds from UPS level to action level`);
|
logger.dim(` - Moving thresholds from UPS level to action level`);
|
||||||
logger.dim(` - Creating default shutdown actions from existing thresholds`);
|
logger.dim(` - Creating default shutdown actions from existing thresholds`);
|
||||||
|
|
||||||
// Migrate UPS devices
|
// Migrate UPS devices
|
||||||
const migratedDevices = (config.upsDevices || []).map((device: any) => {
|
const devices = (config.upsDevices as Array<Record<string, unknown>>) || [];
|
||||||
const migrated: any = {
|
const migratedDevices = devices.map((device) => {
|
||||||
|
const migrated: Record<string, unknown> = {
|
||||||
id: device.id,
|
id: device.id,
|
||||||
name: device.name,
|
name: device.name,
|
||||||
snmp: device.snmp,
|
snmp: device.snmp,
|
||||||
@@ -80,20 +81,21 @@ export class MigrationV4_0ToV4_1 extends BaseMigration {
|
|||||||
};
|
};
|
||||||
|
|
||||||
// If device has thresholds at UPS level, convert to shutdown action
|
// If device has thresholds at UPS level, convert to shutdown action
|
||||||
if (device.thresholds) {
|
const deviceThresholds = device.thresholds as { battery: number; runtime: number } | undefined;
|
||||||
|
if (deviceThresholds) {
|
||||||
migrated.actions = [
|
migrated.actions = [
|
||||||
{
|
{
|
||||||
type: 'shutdown',
|
type: 'shutdown',
|
||||||
thresholds: {
|
thresholds: {
|
||||||
battery: device.thresholds.battery,
|
battery: deviceThresholds.battery,
|
||||||
runtime: device.thresholds.runtime,
|
runtime: deviceThresholds.runtime,
|
||||||
},
|
},
|
||||||
triggerMode: 'onlyThresholds', // Preserve old behavior (only on threshold violation)
|
triggerMode: 'onlyThresholds', // Preserve old behavior (only on threshold violation)
|
||||||
shutdownDelay: 5, // Default delay
|
shutdownDelay: 5, // Default delay
|
||||||
},
|
},
|
||||||
];
|
];
|
||||||
logger.dim(
|
logger.dim(
|
||||||
` → ${device.name}: Created shutdown action (battery: ${device.thresholds.battery}%, runtime: ${device.thresholds.runtime}min)`,
|
` → ${device.name}: Created shutdown action (battery: ${deviceThresholds.battery}%, runtime: ${deviceThresholds.runtime}min)`,
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
// No thresholds, just add empty actions array
|
// No thresholds, just add empty actions array
|
||||||
@@ -104,7 +106,8 @@ export class MigrationV4_0ToV4_1 extends BaseMigration {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// Add actions to groups
|
// Add actions to groups
|
||||||
const migratedGroups = (config.groups || []).map((group: any) => ({
|
const groups = (config.groups as Array<Record<string, unknown>>) || [];
|
||||||
|
const migratedGroups = groups.map((group) => ({
|
||||||
...group,
|
...group,
|
||||||
actions: group.actions || [],
|
actions: group.actions || [],
|
||||||
}));
|
}));
|
||||||
|
@@ -1,7 +1,8 @@
|
|||||||
import process from 'node:process';
|
import process from 'node:process';
|
||||||
import { promises as fs } from 'node:fs';
|
import { promises as fs } from 'node:fs';
|
||||||
import { execSync } from 'node:child_process';
|
import { execSync } from 'node:child_process';
|
||||||
import { NupstDaemon } from './daemon.ts';
|
import { NupstDaemon, type IUpsConfig } from './daemon.ts';
|
||||||
|
import { NupstSnmp } from './snmp/manager.ts';
|
||||||
import { logger } from './logger.ts';
|
import { logger } from './logger.ts';
|
||||||
import { theme, symbols, getBatteryColor, getRuntimeColor, formatPowerStatus } from './colors.ts';
|
import { theme, symbols, getBatteryColor, getRuntimeColor, formatPowerStatus } from './colors.ts';
|
||||||
|
|
||||||
@@ -277,14 +278,23 @@ WantedBy=multi-user.target
|
|||||||
await this.displaySingleUpsStatus(ups, snmp);
|
await this.displaySingleUpsStatus(ups, snmp);
|
||||||
}
|
}
|
||||||
} else if (config.snmp) {
|
} else if (config.snmp) {
|
||||||
// Legacy single UPS configuration
|
// Legacy single UPS configuration (v1/v2 format)
|
||||||
logger.info('UPS Devices (1):');
|
logger.info('UPS Devices (1):');
|
||||||
const legacyUps = {
|
const legacyUps: IUpsConfig = {
|
||||||
id: 'default',
|
id: 'default',
|
||||||
name: 'Default UPS',
|
name: 'Default UPS',
|
||||||
snmp: config.snmp,
|
snmp: config.snmp,
|
||||||
thresholds: config.thresholds,
|
|
||||||
groups: [],
|
groups: [],
|
||||||
|
actions: config.thresholds
|
||||||
|
? [
|
||||||
|
{
|
||||||
|
type: 'shutdown',
|
||||||
|
thresholds: config.thresholds,
|
||||||
|
triggerMode: 'onlyThresholds',
|
||||||
|
shutdownDelay: 5,
|
||||||
|
},
|
||||||
|
]
|
||||||
|
: [],
|
||||||
};
|
};
|
||||||
|
|
||||||
await this.displaySingleUpsStatus(legacyUps, snmp);
|
await this.displaySingleUpsStatus(legacyUps, snmp);
|
||||||
@@ -307,7 +317,7 @@ WantedBy=multi-user.target
|
|||||||
* @param ups UPS configuration
|
* @param ups UPS configuration
|
||||||
* @param snmp SNMP manager
|
* @param snmp SNMP manager
|
||||||
*/
|
*/
|
||||||
private async displaySingleUpsStatus(ups: any, snmp: any): Promise<void> {
|
private async displaySingleUpsStatus(ups: IUpsConfig, snmp: NupstSnmp): Promise<void> {
|
||||||
try {
|
try {
|
||||||
// Create a test config with a short timeout
|
// Create a test config with a short timeout
|
||||||
const testConfig = {
|
const testConfig = {
|
||||||
@@ -332,7 +342,7 @@ WantedBy=multi-user.target
|
|||||||
const batteryColor = getBatteryColor(status.batteryCapacity);
|
const batteryColor = getBatteryColor(status.batteryCapacity);
|
||||||
|
|
||||||
// Get threshold from actions (if any action has thresholds defined)
|
// Get threshold from actions (if any action has thresholds defined)
|
||||||
const actionWithThresholds = ups.actions?.find((action: any) => action.thresholds);
|
const actionWithThresholds = ups.actions?.find((action) => action.thresholds);
|
||||||
const batteryThreshold = actionWithThresholds?.thresholds?.battery;
|
const batteryThreshold = actionWithThresholds?.thresholds?.battery;
|
||||||
const batterySymbol = batteryThreshold !== undefined && status.batteryCapacity >= batteryThreshold
|
const batterySymbol = batteryThreshold !== undefined && status.batteryCapacity >= batteryThreshold
|
||||||
? symbols.success
|
? symbols.success
|
||||||
|
Reference in New Issue
Block a user