Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose moving as motion sensor #956

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Expose moving as motion sensor #956

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf4b05f
Expose moving as motion sensor
torandreroland Nov 16, 2024
a117ab6
Added documentation, test and created changelog entry
torandreroland Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/converters/basic_sensors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { WaterLeakSensorHandler, GasLeakSensorHandler } from './basic_sensors/le
import { CarbonMonoxideSensorHandler } from './basic_sensors/carbon_monoxide';
import { SmokeSensorHandler } from './basic_sensors/smoke';
import { VibrationSensorHandler } from './basic_sensors/vibration';
import { MovingSensorHandler } from './basic_sensors/moving';
import { PresenceSensorHandler } from './basic_sensors/presence';
import { OccupancySensorHandler } from './basic_sensors/occupancy';
import { IdentifierGenerator } from './basic_sensors/basic';
Expand Down Expand Up @@ -52,6 +53,7 @@ export class BasicSensorCreator implements ServiceCreator {
OccupancySensorHandler,
PresenceSensorHandler,
VibrationSensorHandler,
MovingSensorHandler,
SmokeSensorHandler,
CarbonMonoxideSensorHandler,
WaterLeakSensorHandler,
Expand Down
30 changes: 30 additions & 0 deletions src/converters/basic_sensors/moving.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { BasicAccessory } from '../interfaces';
import { ExposesEntryWithBinaryProperty, ExposesEntryWithProperty } from '../../z2mModels';
import { hap } from '../../hap';
import { BinarySensorHandler } from './binary';

export class MovingSensorHandler extends BinarySensorHandler {
public static readonly exposesName: string = 'moving';

constructor(expose: ExposesEntryWithProperty, otherExposes: ExposesEntryWithBinaryProperty[], accessory: BasicAccessory) {
super(
accessory,
expose as ExposesEntryWithBinaryProperty,
otherExposes,
MovingSensorHandler.generateIdentifier,
'Motion Sensor (moving)',
(n, t) => new hap.Service.MotionSensor(n, (MovingSensorHandler.exposesName + ' ' + (t ?? '')).trim()),
hap.Characteristic.MotionDetected,
true,
false
);
itavero marked this conversation as resolved.
Show resolved Hide resolved
}
Comment on lines +9 to +21
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

💡 Codebase verification

Refactor string concatenation to template literals across motion sensor handlers

Found similar string concatenation pattern in other motion sensor handlers. Let's update all instances for consistency:

  • src/converters/basic_sensors/moving.ts: Change to `${MovingSensorHandler.exposesName} ${t ?? ''}`.trim()
  • src/converters/basic_sensors/vibration.ts: Change to `${VibrationSensorHandler.exposesName} ${t ?? ''}`.trim()

The HomeKit service configuration is consistent across all implementations, using the correct characteristic hap.Characteristic.MotionDetected with proper initialization.

🔗 Analysis chain

Refactor string concatenation to use template literals.

The constructor implementation is correct, but we can improve readability by using template literals.

Apply this change:

-      (n, t) => new hap.Service.MotionSensor(n, (MovingSensorHandler.exposesName + ' ' + (t ?? '')).trim()),
+      (n, t) => new hap.Service.MotionSensor(n, `${MovingSensorHandler.exposesName} ${t ?? ''}`.trim()),

Let's verify the HomeKit service configuration:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for consistent motion sensor service usage across the codebase
rg -A 2 "new hap\.Service\.MotionSensor"

Length of output: 861

🧰 Tools
🪛 Biome

[error] 16-16: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)


static generateIdentifier(endpoint: string | undefined) {
let identifier = MovingSensorHandler.exposesName + '_' + hap.Service.MotionSensor.UUID;
if (endpoint !== undefined) {
identifier += '_' + endpoint.trim();
}
return identifier;
}
Comment on lines +23 to +29
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Refactor identifier generation to use template literals.

The identifier generation logic is correct, but we can improve readability by using template literals.

Apply these changes:

-    let identifier = MovingSensorHandler.exposesName + '_' + hap.Service.MotionSensor.UUID;
+    let identifier = `${MovingSensorHandler.exposesName}_${hap.Service.MotionSensor.UUID}`;
     if (endpoint !== undefined) {
-      identifier += '_' + endpoint.trim();
+      identifier = `${identifier}_${endpoint.trim()}`;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
static generateIdentifier(endpoint: string | undefined) {
let identifier = MovingSensorHandler.exposesName + '_' + hap.Service.MotionSensor.UUID;
if (endpoint !== undefined) {
identifier += '_' + endpoint.trim();
}
return identifier;
}
static generateIdentifier(endpoint: string | undefined) {
let identifier = `${MovingSensorHandler.exposesName}_${hap.Service.MotionSensor.UUID}`;
if (endpoint !== undefined) {
identifier = `${identifier}_${endpoint.trim()}`;
}
return identifier;
}
🧰 Tools
🪛 Biome

[error] 24-24: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

[error] 26-26: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

}
Comment on lines +1 to +30
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Implementation successfully meets PR objectives.

The MovingSensorHandler implementation effectively exposes the "moving" state as a motion sensor while following established patterns and HomeKit standards. The code is well-structured and maintainable.

Consider documenting the following in the PR description or README:

  1. The mapping between the "moving" state and motion detection
  2. Any specific behavior or limitations users should be aware of when using this with the SmartThings/Samjin Multipurpose sensor
🧰 Tools
🪛 Biome

[error] 16-16: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

[error] 24-24: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)

[error] 26-26: Template literals are preferred over string concatenation.

Unsafe fix: Use a template literal.

(lint/style/useTemplate)