Skip to content

Commit 2b837ce

Browse files
dougrathbonedougrathbone
committed
Refactor codebase: eliminate ~170 lines through utility consolidation
- Create shared systemUtils.js for install/uninstall service management - Standardize logging across all classes using structured logger - Consolidate settings validation into unified settingsValidator.js - Extract buffer parsing utilities for command/event data processing - Unify Home Assistant discovery patterns for all device types - Remove duplicate code patterns and improve maintainability Major changes: - New utilities: systemUtils.js, settingsValidator.js, bufferParser.js - Updated 9 existing files to use shared functionality - Enhanced error handling and validation consistency
1 parent ece76f0 commit 2b837ce

14 files changed

+423
-244
lines changed

‎.claude/settings.local.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
"Bash(git add:*)",
88
"Bash(git commit:*)",
99
"Bash(npm test)",
10-
"Bash(npm test:*)"
10+
"Bash(npm test:*)",
11+
"Bash(grep:*)",
12+
"Bash(find:*)"
1113
],
1214
"deny": []
1315
}

‎index.js‎

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const path = require('path');
44
const CgateWebBridge = require('./src/cgateWebBridge');
5+
const { validateWithWarnings } = require('./src/settingsValidator');
56

67
// --- Default Settings (can be overridden by ./settings.js) ---
78
const defaultSettings = {
@@ -46,35 +47,13 @@ try {
4647
// Merge settings
4748
const settings = { ...defaultSettings, ...userSettings };
4849

49-
// Validate critical settings
50-
function validateSettings() {
51-
const errors = [];
52-
53-
if (!settings.cbusip) {
54-
errors.push('cbusip is required');
55-
}
56-
57-
if (!settings.cbusname) {
58-
errors.push('cbusname is required');
59-
}
60-
61-
if (!settings.mqtt) {
62-
errors.push('mqtt broker address is required');
63-
}
64-
65-
if (errors.length > 0) {
66-
console.error('[ERROR] Invalid configuration:');
67-
errors.forEach(error => console.error(` - ${error}`));
68-
process.exit(1);
69-
}
70-
}
7150

7251
// Application startup
7352
function main() {
7453
console.log('[INFO] Starting cgateweb...');
7554
console.log(`[INFO] Version: ${require('./package.json').version}`);
7655

77-
validateSettings();
56+
validateWithWarnings(settings);
7857

7958
// Create and start the bridge
8059
const bridge = new CgateWebBridge(settings);

‎install-service.js‎

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,34 +2,14 @@
22

33
const fs = require('fs');
44
const path = require('path');
5-
const { execSync } = require('child_process');
5+
const { runCommand, checkRoot } = require('./src/systemUtils');
66

77
const SERVICE_NAME = 'cgateweb.service';
88
const SOURCE_SERVICE_FILE_TEMPLATE = path.join(__dirname, 'cgateweb.service.template');
99
const TARGET_SYSTEMD_DIR = '/etc/systemd/system';
1010
const TARGET_SERVICE_FILE = path.join(TARGET_SYSTEMD_DIR, SERVICE_NAME);
1111
const BASE_INSTALL_PATH = __dirname;
1212

13-
function runCommand(command) {
14-
try {
15-
console.log(`Executing: ${command}`);
16-
execSync(command, { stdio: 'inherit' });
17-
console.log(`Successfully executed: ${command}`);
18-
return true;
19-
} catch (error) {
20-
console.error(`Failed to execute command: ${command}`);
21-
console.error(error.stderr ? error.stderr.toString() : error.message);
22-
return false;
23-
}
24-
}
25-
26-
function checkRoot() {
27-
if (process.getuid && process.getuid() !== 0) {
28-
console.error('This script requires root privileges to copy files to /etc and manage systemd.');
29-
console.error('Please run using sudo: sudo node install-service.js');
30-
process.exit(1);
31-
}
32-
}
3313

3414
function checkDependencies() {
3515
console.log('Checking dependencies...');
@@ -72,7 +52,7 @@ function checkDependencies() {
7252
function installService() {
7353
console.log('--- cgateweb Systemd Service Installer ---');
7454

75-
checkRoot();
55+
checkRoot('install-service.js');
7656
checkDependencies();
7757

7858
// 1. Check if source service file template exists

‎src/bufferParser.js‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
const { NEWLINE } = require('./constants');
2+
3+
/**
4+
* Utility for parsing newline-delimited stream data
5+
* Handles common pattern of accumulating data in a buffer and processing complete lines
6+
*/
7+
class BufferParser {
8+
constructor(options = {}) {
9+
this.buffer = '';
10+
this.delimiter = options.delimiter || NEWLINE;
11+
this.trimLines = options.trimLines !== false; // Default to true
12+
this.skipEmptyLines = options.skipEmptyLines !== false; // Default to true
13+
}
14+
15+
/**
16+
* Add new data to the buffer and process any complete lines
17+
* @param {Buffer|string} data - New data to add
18+
* @param {function} lineProcessor - Function to call for each complete line
19+
*/
20+
processData(data, lineProcessor) {
21+
if (typeof lineProcessor !== 'function') {
22+
throw new Error('lineProcessor must be a function');
23+
}
24+
25+
// Convert buffer to string and add to internal buffer
26+
this.buffer += data.toString();
27+
28+
// Process all complete lines
29+
let delimiterIndex;
30+
while ((delimiterIndex = this.buffer.indexOf(this.delimiter)) > -1) {
31+
let line = this.buffer.substring(0, delimiterIndex);
32+
this.buffer = this.buffer.substring(delimiterIndex + this.delimiter.length);
33+
34+
// Apply line processing options
35+
if (this.trimLines) {
36+
line = line.trim();
37+
}
38+
39+
if (this.skipEmptyLines && !line) {
40+
continue;
41+
}
42+
43+
// Process the complete line
44+
try {
45+
lineProcessor(line);
46+
} catch (error) {
47+
// Re-throw with additional context
48+
throw new Error(`Error processing line "${line}": ${error.message}`);
49+
}
50+
}
51+
}
52+
53+
/**
54+
* Get the current buffer contents (incomplete line data)
55+
* @returns {string} - Current buffer contents
56+
*/
57+
getBuffer() {
58+
return this.buffer;
59+
}
60+
61+
/**
62+
* Clear the buffer
63+
*/
64+
clearBuffer() {
65+
this.buffer = '';
66+
}
67+
68+
/**
69+
* Check if buffer has any data
70+
* @returns {boolean} - True if buffer is not empty
71+
*/
72+
hasData() {
73+
return this.buffer.length > 0;
74+
}
75+
76+
/**
77+
* Process any remaining data in buffer as final line (useful for cleanup)
78+
* @param {function} lineProcessor - Function to call for remaining data
79+
*/
80+
processFinalLine(lineProcessor) {
81+
if (this.hasData()) {
82+
let line = this.buffer;
83+
if (this.trimLines) {
84+
line = line.trim();
85+
}
86+
if (!this.skipEmptyLines || line) {
87+
lineProcessor(line);
88+
}
89+
this.clearBuffer();
90+
}
91+
}
92+
}
93+
94+
/**
95+
* Convenience function for simple line-by-line processing
96+
* @param {Buffer|string} data - Data to process
97+
* @param {function} lineProcessor - Function to call for each line
98+
* @param {Object} options - Parser options
99+
*/
100+
function processLines(data, lineProcessor, options = {}) {
101+
const parser = new BufferParser(options);
102+
parser.processData(data, lineProcessor);
103+
104+
// Process any remaining data as final line if requested
105+
if (options.processFinalLine) {
106+
parser.processFinalLine(lineProcessor);
107+
}
108+
109+
return parser.getBuffer(); // Return any remaining buffer data
110+
}
111+
112+
module.exports = {
113+
BufferParser,
114+
processLines
115+
};

‎src/cbusCommand.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
const { createLogger } = require('./logger');
12
const {
23
COMMAND_TOPIC_REGEX,
34
MQTT_CMD_TYPE_GETALL,
@@ -52,12 +53,13 @@ class CBusCommand {
5253
this._commandType = null;
5354
this._level = null;
5455
this._rampTime = null;
56+
this._logger = createLogger({ component: 'CBusCommand' });
5557

5658
if (this._topic) {
5759
this._parse();
5860
} else {
5961
// Handle empty/null topic
60-
console.warn(`${ERROR_PREFIX} Empty MQTT command topic`);
62+
this._logger.warn(`Empty MQTT command topic`);
6163
this._parsed = true;
6264
this._isValid = false;
6365
}
@@ -67,7 +69,7 @@ class CBusCommand {
6769
try {
6870
const match = this._topic.match(COMMAND_TOPIC_REGEX);
6971
if (!match) {
70-
console.warn(`${ERROR_PREFIX} Invalid MQTT command topic format: ${this._topic}`);
72+
this._logger.warn(`Invalid MQTT command topic format: ${this._topic}`);
7173
this._isValid = false;
7274
this._parsed = true;
7375
return;
@@ -81,7 +83,7 @@ class CBusCommand {
8183
// Validate command type
8284
const validCommandTypes = [MQTT_CMD_TYPE_GETALL, MQTT_CMD_TYPE_GETTREE, MQTT_CMD_TYPE_SWITCH, MQTT_CMD_TYPE_RAMP, 'setvalue'];
8385
if (!validCommandTypes.includes(this._commandType)) {
84-
console.warn(`${ERROR_PREFIX} Invalid MQTT command type: ${this._commandType}`);
86+
this._logger.warn(`Invalid MQTT command type: ${this._commandType}`);
8587
this._isValid = false;
8688
this._parsed = true;
8789
return;
@@ -93,7 +95,7 @@ class CBusCommand {
9395
this._isValid = true;
9496
this._parsed = true;
9597
} catch (error) {
96-
console.error(`${ERROR_PREFIX} Error parsing MQTT command topic: ${this._topic}`, error);
98+
this._logger.error(`Error parsing MQTT command topic: ${this._topic}`, { error });
9799
this._isValid = false;
98100
this._parsed = true;
99101
}

‎src/cbusEvent.js‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
const { createLogger } = require('./logger');
12
const { EVENT_REGEX, CGATE_RESPONSE_OBJECT_STATUS, ERROR_PREFIX } = require('./constants');
23

34
/**
@@ -38,12 +39,13 @@ class CBusEvent {
3839
this._application = null;
3940
this._group = null;
4041
this._isValid = false;
42+
this._logger = createLogger({ component: 'CBusEvent' });
4143

4244
if (this._rawEvent) {
4345
this._parse();
4446
} else {
4547
// Handle empty input
46-
console.warn(`${ERROR_PREFIX} Empty C-Bus event data`);
48+
this._logger.warn(`Empty C-Bus event data`);
4749
this._parsed = true;
4850
this._isValid = false;
4951
}
@@ -61,7 +63,7 @@ class CBusEvent {
6163
const match = this._rawEvent.match(EVENT_REGEX);
6264
if (!match) {
6365
// Not a recognizable event format
64-
console.warn(`${ERROR_PREFIX} Could not parse C-Bus event: ${this._rawEvent}`);
66+
this._logger.warn(`Could not parse C-Bus event: ${this._rawEvent}`);
6567
this._isValid = false;
6668
return;
6769
}
@@ -81,17 +83,17 @@ class CBusEvent {
8183
this._group = addressParts[2];
8284
this._isValid = true;
8385
} else {
84-
console.warn(`${ERROR_PREFIX} Invalid C-Bus address format: ${this._address}`);
86+
this._logger.warn(`Invalid C-Bus address format: ${this._address}`);
8587
this._isValid = false;
8688
}
8789
} else {
88-
console.warn(`${ERROR_PREFIX} Missing address in C-Bus event: ${this._rawEvent}`);
90+
this._logger.warn(`Missing address in C-Bus event: ${this._rawEvent}`);
8991
this._isValid = false;
9092
}
9193

9294
this._parsed = true;
9395
} catch (error) {
94-
console.error(`${ERROR_PREFIX} Error parsing C-Bus event: ${this._rawEvent}`, error);
96+
this._logger.error(`Error parsing C-Bus event: ${this._rawEvent}`, { error });
9597
this._isValid = false;
9698
this._parsed = true;
9799
}
@@ -119,7 +121,7 @@ class CBusEvent {
119121
}
120122
} else {
121123
// Invalid status response format
122-
console.warn(`${ERROR_PREFIX} Invalid status response format: ${this._rawEvent}`);
124+
this._logger.warn(`Invalid status response format: ${this._rawEvent}`);
123125
this._isValid = false;
124126
}
125127

0 commit comments

Comments
 (0)