Repository navigation
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7dc907a5d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #define SUPLA_SC_CALL_DEVICE_CONFIG_UPDATE_OR_RESULT 1250 // ver. >= 21 | ||
| #define SUPLA_DS_CALL_SET_SUBDEVICE_DETAILS 1260 // ver. >= 25 | ||
|
|
||
| #define SUPLA_SD_CALL_GET_OBJECT_ALERT_CAPABILITIES 1270 // ver. >= 29 |
There was a problem hiding this comment.
Register object-alert calls in SRPC
The object-alert call IDs introduced here are not added to srpc_call_min_version_required or srpc_getdata in supla-common/srpc.c. For protocol-29 peers using the normal SRPC path, srpc_call_min_version_required falls through to 255 and srpc_async__call rejects the call through srpc_call_allowed, so the new request/result/change messages cannot be sent or decoded even though they are advertised as version 29.
Useful? React with 👍 / 👎.
| * Server <-> Client uses server-side identifiers. | ||
| ********************************************/ | ||
|
|
||
| #define SUPLA_OBJECT_TYPE_NONE 0 |
There was a problem hiding this comment.
In supla, there's something called SUPLA_TARGET_. It might be worth providing similar identifiers. There's also _callerType_e. There are also identifiers there that you could try to share.
There was a problem hiding this comment.
Yes, I'll replace SUPLA_OBJECT_TYPE_ with SUPLA_TARGET_ and I'll extend SUPLA_TARGET_ with SUBDEVICE variant.
"_callerType_e" seems to be something supla-server internal. It also has a little bit different semantics - it contain information about who called some action (i.e. scene, MQTT, etc) and not what is the target object type in our model.
| unsigned char Type; // SUPLA_OBJECT_TYPE_* | ||
| unsigned char Number; // DEVICE: 0, SUBDEVICE: SubDeviceId, CHANNEL: number | ||
| unsigned _supla_int16_t Reserved; | ||
| } TSuplaObjectAddress; // Device <-> Server |
There was a problem hiding this comment.
Here too it seems that the object address has some correlation with _callerType_e
There was a problem hiding this comment.
that's the same "object type" as defines above, so this will be changed to Target
| // Current alert state flags. | ||
| #define SUPLA_ALERT_STATE_ACTIVE (1 << 0) | ||
|
|
||
| // Object alert pack flags. |
There was a problem hiding this comment.
Why can't this work like with channels? Upon registration, we send the entire list of alarms along with their status, and then update the statuses?
There was a problem hiding this comment.
FULL_SNAPSHOT will be send by device after successful registration and will send all active alarms (or empty list if there is no active alarm), so it's kind of similar to standard channel state sync during registration.
Or do you mean to send all active and inactive alarms?
after registration, then it sends only updates. Removed capability fetch (it is replaced with a full report send after registration). Add command to explicitly call "clear latch" on alarm. Add capability flag that enables/disabled admin auth required when reset/clear latch is used from app.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26ad37930b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case SUPLA_DS_CALL_OBJECT_ALERTS_REPORT: | ||
| case SUPLA_DS_CALL_OBJECT_ALERTS_CHANGED: |
There was a problem hiding this comment.
Expose alert reports to client-only builds
If object-alert reports are intended to travel Server<->Client (the new target address comments and Id field document that path in proto.h), keeping these report/changed parsers inside the device-only section leaves Android/client-only SRPC builds unable to receive them because srpc.h defines SRPC_EXCLUDE_DEVICE for __ANDROID__. In that environment srpc_getdata has no case for these call IDs and alert updates sent to clients are rejected as data errors; move the parser to a client-visible case or add proper SC call IDs/helpers.
Useful? React with 👍 / 👎.
| break; | ||
| case SUPLA_DS_CALL_OBJECT_ALERTS_REPORT: | ||
| case SUPLA_DS_CALL_OBJECT_ALERTS_CHANGED: | ||
| if (srpc->sdp.data_size == sizeof(TDS_ObjectAlerts)) { |
There was a problem hiding this comment.
Validate object-alert item counts
For a malformed object-alert packet with Count > SUPLA_OBJECT_ALERT_STATE_PACK_MAXCOUNT, this branch still accepts the message because it only checks the fixed struct size. The packet only carries 60 Items, so passing an oversized count to the remote-call handler makes downstream consumers prone to iterating past the array; reject counts above the max here (and mirror the guard in the send helpers) before returning a valid ds_object_alerts.
Useful? React with 👍 / 👎.
Simplify "Alarm Objects" proposal.
file (temperarly - may be removed later)
Object Alerts
This PR introduces a new Object Alerts subsystem that provides a unified way to report alarms, warnings, faults, maintenance notifications and diagnostic events for SUPLA devices.
Alerts can be associated with any SUPLA object:
Each alert is identified by a stable 16-bit alert code and may expose additional runtime information such as severity, and supported capabilities.
The model supports two alert types:
After device registration or reconnection, the device automatically synchronizes the current alert state by sending full alert REPORT for all alert-capable objects. This allows the server to rebuild the complete alert state without additional requests.
Subsequent alert activations, clear events and occurrence events are reported incrementally using dedicated Object Alerts CHANGE messages.
Alert definitions are organized into protocol-defined categories such as System, Sensor, Output, Calibration, Maintenance, Process, Environment, Safety, Power, Protection and Device. A dedicated range of alert identifiers is reserved for vendor-specific extensions.
The subsystem is designed to support device diagnostics, maintenance workflows, safety monitoring and automation triggers using a single protocol model shared across all device types.
Server-side responsibilities
The server is responsible for maintaining alert state, exposing alerts through APIs, generating automation events and handling alert synchronization.
Core responsibilities:
Store alert capabilities for each object (
DEVICE,SUBDEVICE,CHANNEL).Maintain current active alert state for every object.
Process full alert REPORT received after device registration.
Detect alert activations and clear events by comparing subsequent messages.
Generate automation triggers:
Support alert reset operations for alerts marked as
RESET_SUPPORTED.Expose active alerts through server APIs.
Associate object alerts with
AlertSurfaceChannelNumberto allow presentation in channel-oriented clients.Client applications should be able to display active alerts, alert history (? tbd) and available alert-based automation triggers without requiring any device-specific knowledge.