-
Notifications
You must be signed in to change notification settings - Fork 56
Add unchanged_limit_sec and rulesets to metadata #1201
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
base: master
Are you sure you want to change the base?
Conversation
grafnu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also still pending trying to understand why the tests are failing!
schema/model_policies.json
Outdated
| "$schema": "http://json-schema.org/draft-04/schema#", | ||
| "additionalProperties": false, | ||
| "properties": { | ||
| "named_policies": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_policies seems redundant and named is too short -- how about rule_sets?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, It was purposefully redundant to allow future flexibility because this is a free entry, and maybe there is something more structured - e.g. your point about there being different types of policies or an evolution of it.
rule_sets could work - my current terminology is that there are rules (e.g. device is not sending excessive data), and policies is a combination of rules. The present rules are likely (ip - basically device online/offline , mqtt basic connectivities e.g. device is connected, sending messages, not excessive data, and telemetry which is that pointset events pass udmi validation, point values are within error bounds and stale threshold) - and maybe one around gateways.
schema/model_pointset_point.json
Outdated
| "type": "number", | ||
| "examples": [100] | ||
| }, | ||
| "stale_threshold_sec": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to double check -- is this really a point property or should it be on the entire pointset? Is there actually a use-case for different points having different stale thresholds? I would expect that either the device can be accessed so all points would update or none of them would... ?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say it's on a point, the scenarios would be that the sensor connected to the device is faulty, or a point is misconfigured, or there is some other error of sorts.
There are scenarios where it would differ for valid reasons, for examplle an energy meter fr a lift might not see any usage over the weekend, and it's valid for it to be "stale" for a few days. The main energy meter on the incoming power to a building should probably never be stale.
@pisuke might have thoughts. I think we're setting this on a key points rather than all points on a device.
|
So, theoretically we can have both -- but what's the more common or simpler
case? Seems to me a LOT simpler just to have one for everything -- I just
can't see a world where we usually care enough about all this to warrant
wanting to fiddle with it at each point. My suggestion would be to maybe
start with a blanket pointset level thing (like the frequency of update
parameter), and then if we actually have motivating use cases we can add
the point-level override later?
A lift not getting usage over the weekend isn't a good example -- since in
that case it would be the same value sent multiple times (e.g. it just
keeps sending 0) -- that's not stale. Stale means "could not be updated so
might be wrong" -- so also makes me extra twitchy to introduce so much
overhead of granularity when the use case is fairly speculative!
…On Thu, Oct 30, 2025 at 3:52 PM Noureddine ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In schema/model_pointset_point.json
<#1201 (comment)>:
> @@ -47,6 +47,11 @@
"type": "number",
"examples": [100]
},
+ "stale_threshold_sec": {
I would say it's on a point, the scenarios would be that the sensor
connected to the device is faulty, or a point is misconfigured, or there is
some other error of sorts.
There are scenarios where it would differ for valid reasons, for examplle
an energy meter fr a lift might not see any usage over the weekend, and
it's valid for it to be "stale" for a few days. The main energy meter on
the incoming power to a building should probably never be stale.
@pisuke <https://github.com/pisuke> might have thoughts. I think we're
setting this on a key points rather than all points on a device.
—
Reply to this email directly, view it on GitHub
<#1201 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIEPD7DU5RD7SD2GRIUNH332KJB3AVCNFSM6AAAAACJ5AJAG2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTIMBRHE2DENJSGQ>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
Comments incorporated - although should it be |
|
I'm going to decide not to care too much! Conceptually it should be
`rule_sets` I think. We can just use that and then tell ourselves it's OK
because `pointset` is at the top level so is somehow different/special.
…On Wed, Nov 5, 2025 at 2:30 PM Noureddine ***@***.***> wrote:
*noursaidi* left a comment (faucetsdn/udmi#1201)
<#1201 (comment)>
Comments incorporated - although should it be rule_sets or rulesets A
question recently came up on pointset, so I'm not sure if it;s
inconsistent with that *set* or if it matters at all
—
Reply to this email directly, view it on GitHub
<#1201 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIEPDZQO53DOPA7L742X3T33J3AHAVCNFSM6AAAAACJ5AJAG2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIOJTHA3DCMBTGU>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
grafnu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one nitty thing about the name to be singular!
| } | ||
| } | ||
| }, | ||
| "policies": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, time to be a pedant -- this should just be policy -- the plural form generally indicates that the thing is a { set, list, map } -- a collection of some sort. This level is just a container object, so it's singular.
No description provided.