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.

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

feat: Add support for Sector Alarm event entities and log ingestion #218

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

Conversation

garnser
Copy link
Collaborator

@garnser garnser commented Nov 7, 2024

  • Implemented SectorAlarmEvent entity to track log events per device
  • Established association between logs and pre-existing lock devices
  • Resolved name and model overriding issues when adding event entities
  • Parsed timestamps to compatible ISO format for Home Assistant display
  • Enabled log entries to be visible in the Home Assistant logbook
  • Added fallback setup and debug logging for reliable event ingestion

This update introduces event tracking for Sector Alarm, enhancing log visibility and historical tracking directly in Home Assistant.

fixes #220

- Implemented SectorAlarmEvent entity to track log events per device
- Established association between logs and pre-existing lock devices
- Resolved name and model overriding issues when adding event entities
- Parsed timestamps to compatible ISO format for Home Assistant display
- Enabled log entries to be visible in the Home Assistant logbook
- Added fallback setup and debug logging for reliable event ingestion

This update introduces event tracking for Sector Alarm, enhancing log visibility and historical tracking directly in Home Assistant.
@garnser garnser added the new-feature New features or options. label Nov 7, 2024
@garnser garnser linked an issue Nov 7, 2024 that may be closed by this pull request
custom_components/sector/__init__.py Outdated Show resolved Hide resolved
custom_components/sector/camera.py Outdated Show resolved Hide resolved
custom_components/sector/event.py Outdated Show resolved Hide resolved
custom_components/sector/event.py Outdated Show resolved Hide resolved
custom_components/sector/lock.py Outdated Show resolved Hide resolved
@gjohansson-ST
Copy link
Owner

Should the event entity not be for thing which are not covered by the other entities.
Does it provide additional value to see both on the lock and an event that it's locking?

@garnser
Copy link
Collaborator Author

garnser commented Nov 7, 2024

Should the event entity not be for thing which are not covered by the other entities. Does it provide additional value to see both on the lock and an event that it's locking?

One event which isn't picked up by the lock entity is "lock_failed" which is quite crucial if someone has left the door in a semi-closed state and the lock can't engage.

For events such as arming/disarming the logs include data about what user and how the alarm was engaged which isn't otherwise accessible.

Alternatively we could just filter out the things which we can only get from the log.

@gjohansson-ST
Copy link
Owner

One event which isn't picked up by the lock entity is "lock_failed" which is quite crucial if someone has left the door in a semi-closed state and the lock can't engage.

I think we should update lock platform with is_jammed for this.

For events such as arming/disarming the logs include data about what user and how the alarm was engaged which isn't otherwise accessible.

That makes sense 👍

Alternatively we could just filter out the things which we can only get from the log.

I think with the event start small and then increase the scope one by one.
Maybe just do one event entity to start with for alarm_changed_by to get a working example and then we expand it from there?

custom_components/sector/coordinator.py Outdated Show resolved Hide resolved
custom_components/sector/event.py Outdated Show resolved Hide resolved
custom_components/sector/coordinator.py Outdated Show resolved Hide resolved
custom_components/sector/coordinator.py Outdated Show resolved Hide resolved
Jonathan Petersson added 6 commits November 8, 2024 14:36
- Corrected the matching logic in `process_events` within `coordinator.py`:
  - Previously, logs' `LockName` values were not matching any devices because `self.data["devices"]` used serial numbers as keys.
  - Updated the code to iterate over `self.data["devices"]` and match `lock_name` against each device's `name` field.
  - This ensures that events are correctly associated with their respective devices based on `LockName`.

- Resolved `RuntimeWarning` about `async_update` not being awaited in `event.py`:
  - Adjusted the listener setup in `async_added_to_hass` to properly schedule `async_update` as a task.
  - Used `lambda: self.hass.async_create_task(self.async_update())` to ensure `async_update` is awaited.

- Modified the `state` property in `SectorAlarmEvent` to return the latest `EventType` instead of the timestamp:
  - This aligns the entity state with the expected event type (e.g., "lock", "unlock").

- Cleaned up redundant code and unused attributes in `event.py`:
  - Removed `_queued_events` and related logic since events are processed immediately.
  - Simplified the class by keeping only the necessary attributes.

- Added additional debug logging throughout `coordinator.py` and `event.py`:
  - Included logs for device initialization, event processing, and entity creation.
  - These logs aid in tracing the flow of data and diagnosing issues in event handling.

These changes address the issue where lock events were not being matched to devices due to incorrect key usage in device mapping. They also fix asynchronous handling in the event listener to prevent runtime warnings.

---

**Files Changed:**

- `coordinator.py`:
  - Updated `process_events` to match `LockName` with devices by `name` field.
  - Added debug statements to trace event processing.
  - Removed unused methods and cleaned up the code.

- `event.py`:
  - Fixed asynchronous call to `async_update` by scheduling it properly.
  - Changed `state` property to return the latest event type.
  - Removed redundant attributes and methods.
  - Enhanced debug logging for entity creation and event triggering.
…ual event history instead, cache the result to avoid multiple lookups towards the API
@garnser
Copy link
Collaborator Author

garnser commented Nov 8, 2024

I think we should update lock platform with is_jammed for this.

So rather than registering failed_lock as an event should it update the state of the lock entity?

@garnser
Copy link
Collaborator Author

garnser commented Nov 11, 2024

Ready for review again, everything is working as expected now.

@gjohansson-ST gjohansson-ST marked this pull request as draft November 13, 2024 20:08
@garnser
Copy link
Collaborator Author

garnser commented Nov 28, 2024

@gjohansson-ST can you review please, it's working as it should now.

@gjohansson-ST gjohansson-ST marked this pull request as ready for review December 8, 2024 14:53
Comment on lines +39 to +48
# Define unique entity ID for each device, regardless of event type
entity_unique_id = f"{device_serial}_event"

# Check if the entity already exists by unique ID
if hass.states.get(f"event.{entity_unique_id}"):
_LOGGER.debug(
"SECTOR_EVENT: Entity with unique ID '%s' already exists, skipping.",
entity_unique_id,
)
continue
Copy link
Owner

Choose a reason for hiding this comment

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

I think we should just replace here to add an event entity for each lock and then it's done.

"SECTOR_EVENT: Returning state for device %s: %s",
self._serial_no,
self._last_event_type or "No events",
)
return self._last_event_type or "No events"
Copy link
Owner

Choose a reason for hiding this comment

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

state should return the last event time, now it's returning the event type (so it's no longer an event entity).
We need to overwrite __last_event_triggered in the handling as otherwise, it won't be the correct time.

"manufacturer": "Sector Alarm",
"model": self._device_info["model"],
}
def unique_id(self):
Copy link
Owner

Choose a reason for hiding this comment

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

Remove as it's covered by the parent class

event_timestamp = event_attributes.get(
"Time", datetime.now(timezone.utc).isoformat()
"timestamp", datetime.now(dt_util.UTC).isoformat()
Copy link
Owner

Choose a reason for hiding this comment

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

I think we should just set the correct time in __last_event_triggered as mentioned also below, then remove the formatting etc. and let it be done by the built in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature New features or options.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve logging of locks Expose logs as events
2 participants