-
Notifications
You must be signed in to change notification settings - Fork 3
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
ITC-3 uses incorrect timestamp for traffic counting #454
Comments
The validator doesn't catch this issue. Perhaps the test case can be extended |
The schema for S0207 should catch this, as the 'n' field in a sS 'start' element must be a standard rsmp timestamp: But it seems this is not working for some reason. |
I think the timestamp format is valid enough, I'm guessing Davids suggestion is that this test controller indeed has not been counting since 1970? |
ok, so the schema is correct, but the device is reporting on incorrect datetime |
The RSMP program logs a timestamp whenever a detection is made. If there are no detections, then it defaults to the 1970 timestamp. What timestamp do you propose to put in, when there haven't been made any detections yet? |
The "start" attribute is defined as "Time stamp for start of measuring", not when the first detection was made. Start of measurement is when the last update was send. I think that should cover all cases, since you cannot get a status update without first subscribing, and when you subscribe you first get a status update. |
That makes sense. So, in every case, the timestamp just reflects when RSMP transmits a status update, either as a response to a status request or a subscription. |
Yes. I think it could be clearer in the docs. @otterdahl? |
Yes. I think it could be clearer in the docs. @otterdahl?
Sure. Right now it it mostly just says "Time stamp for start of measuring" and this could be clarified further.
But please note that we also have Section 4: "Traffic data": https://rsmp-nordic.github.io/rsmp_sxl_traffic_lights/1.2.1/traffic_data.html where we try to explain this.
Perhaps we should add "Time stamp for start of measuring. Updated when a status update/response is sent.".
But historically we've deliberately avoided to define that is should update when it's sent since this type
of message could be buffered and in that case it might be better to refer to the measuring period as
mentioned in section 4.
|
I wasn't aware of this section 4. I find some of the requirements in section 4 somewhat surprising, or at least very specific in the sense that they affect the delivery mechanism defined in the core spec, e.g. don't send an initial status update, send only on even intervals. It might be helpful to add a link to the section 4 in the section of each status message related to traffic data? E.g. in: |
Sure. I've opened a PR: rsmp-nordic/rsmp_schema#124 |
Would it be possible to design 201-208 in a way that does not violate the RSMP Core specification? I.e. 201-208 requires that no initial update is sent, vs. the Core spec stating that an immediate update message should be sent upon subscription. Our RSMP implementation has never supported this violation, and we have heard no complaints about this so far, for the more than 10 years the implementation has existed. In my opinion, it is dangerous to have such violations of the core spec. It is confusing both for implementers and consumers, and it makes it more difficult to validate that the core spec is adhered to. Especially in this case, where the documentation was not correct. So I would like to discuss if we can remove this "violation". Is there any danger to sending out the initial data? In my opinion it is the right thing to do. For example, let's say that there has been no queries or subscriptions, up until now. RSMP has detected 102 vehicles since the daemon was started. Now a subscription is initiated, for lets say every 5 minutes. Upon subscription, the RSMP daemon will immediately send a StatusUpdate message back, containing the 102 vehicles detected so far, with a timestamp equal to "now" (just as if a normal query was performed). From then on, StatusUpdates will proceed as normal, containing only data since last interval. This all sounds "correct" to me. You even have this statement in the documentation (https://rsmp-nordic.github.io/rsmp_core/3.1.5/applicability/basic_structure.html#status-messages): The reason for sending the response immediately is because subscriptions usually are established shortly after RSMP connection establishment and the supervision system needs to update with the current statuses and events. |
The reason why we added this exception was due to a potential edge case right when the subscription starts. As far as I understand, traffic counting data is only truly useful if it all detectors start measuring at the same time and measures during even time intervals, (e.g. 15:00:00- 15:05:00). If this is not the case than limit what kind of analysis we can perform when we compare traffic data between detectors/controllers. We were concerned that if the send an initial StatusUpdate upon subscription start, then we might include partial or too much data (e.g. 15:00:00-15:03:23 or 14:23:23-15:00:00). But as you mention, this is a edge case that only applies to the start of subscription and after that it really no problem. I guess that the supervision system might as well filter out any initial data where the measuring period exceeds or deviates from the expectation. So if this exception causes issues then I think we can consider it's removal. |
I think I have found a special case, that would require some clarification: Say that a subscription has been set up for S0201. Every 5 minutes., starting from 15:00 Now, a get request is issued to S0201 at 15:02. What data should this get-request return? Should it return the counts from 15:00 to 15:02 and contain a starttime=15:00? What then happens to the subscription that has been set up? Should it still return the data from 15:00 to 15:05? Or should it be reset to return from 15:02 to 15:05? |
Should it return the counts from 15:00 to 15:02 and contain a starttime=15:00?
Yes, I think so.
What then happens to the subscription that has been set up? Should it still return the data from 15:00 to 15:05?
Yes. I think the subscription should be unaffected.
Or should it be reset to return from 15:02 to 15:05?
No. As stated in section 4 "Traffic data" "The traffic counter may only reset its traffic counter at even time intervals."
|
Nice. Thank you for the clarification. So it sounds like get-requests during a subscription cannot "reset" or interfere with the subscription. I.e. it is merely peeking. What happens to get-requests when there is no subscription? Should they also never reset the count? |
Nice. Thank you for the clarification. So it sounds like get-requests during a subscription cannot "reset" or interfere with the subscription. I.e. it is merely peeking. What happens to get-requests when there is no subscription? Should they also never reset the count?
Correct. I don't think there should be any change of behavior of
get-requests depending on if subscription is active or not.
|
I think is an important core-spec discussion, opened rsmp-nordic/rsmp_core#213. |
I'm also in favour of removing any requirement in the SXL that override core specs. |
Based on dicussions above, I think very old timestamps (1970) could be flagged as an error in the validator? |
Another issue I found regarding S0201-S0208: What happens if you only subscribe partially to an object? Say for example that you only subscribed to the "vehicles" object in S0201, and not "starttime" (unlikely, I know)? This is just to prove another point, that it is bad that some parts of the SXL have special requirements and violate the core spec. All these special cases listed here: https://rsmp-nordic.github.io/rsmp_sxl_traffic_lights/1.0.15/traffic_data.html requires that you subscribe to the entire SXL object, and not just sub-parts of it. I think that this should be stated clearly on the link above, and that partial subscriptions will not be supported, or at least, lead to undefined behavior. Alternatively, it should be stated that subscribing to sub-parts of an object in S0201-S0208, enables the "traffic data" special cases, for all sub-objects, i.e. their values may change even though you did not subscribe to them. Also I think it would be good to disable "subscribe-on-change" for these objects. I.e. you cannot make both an interval subscription and subscribe-on-change. |
Attributes that must always to subscribe to together could perhaps be send as a single json object: {
"n": "data",
"s": { "vehicles": 45, "starttime": "2019-09-26T12:48:00.285Z" }
} |
This is already the case. Elements can be subscribed to as a single json message, and will be replied back as a single message, if they have the same subscription interval. My point is, there is nothing forcing a user to do this. You could legally set up a subscription for "vehicles" alone, for example. But it is currently undefined what then happens to the "starttime" field, as there is no subscription to it. |
As you may have noticed by now, I have been making a lot of posts about S0201-S0208 specifically, to point out all the discrepancies, inconsitencies and undefined parts. This is to prove a point that the current specification in https://rsmp-nordic.github.io/rsmp_sxl_traffic_lights/1.0.15/traffic_data.html is insufficient. Perhaps it should be boiled down to its most basic ideas, and then leave the rest of the edge cases as "undefined behavior". For example, the page could just state:
ALL other cases are undefined, and should not be used (i.e. Do not query, do not change subscription interval while subscription is running, e.t.c.). I think this would make it more clear what the intent of these objects are, and much easier to implement both on the server and in the clients. |
In recent runs, ITC-3 reports an incorrect time stamp
1970-01-01T00:00:00.000Z
for start of traffic counting (S0201-S0208)Example
The text was updated successfully, but these errors were encountered: