-
Notifications
You must be signed in to change notification settings - Fork 23
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
AIS decoding problem #324
Comments
Could you provide a log of test data with the problem? |
Thanks for the quick reply. What do you mean by log? The input data stream or the NmeaLogger?
|
Side note: When switching to SK as input it is not sufficient to disable the SocketReader and to enable the SignalKHandler. It won't pick up the ais data. I have to restart AvNav to get the data in via SK. |
The logs are what I need .. For the switching problem: |
I found one error in my code, the padding of the type 5 messages was wrong, so there were some extra trailing zero bits. The SK decoder didn't care for that and AvNav did also decode some of these messages correctly and displayed the vessel's names. Now with this fixed, all ais target show their names, but still not all of the targets are displayed, some are still missing. Only 8 of 10 ais targets are displayed. |
If I omit the type 5 messages and only send type 1, then all targets are displayed (w/o static info of course). So, I don't think that the type 1 messages are malformed. There are missing target when type 1 and type 5 messages are submitted together. all of the 10 targets show up |
OpenCPN also handles the input as expected and shows all of the 10 targets. |
Which targets are missing seems to depend on the order of the AIS NMEA sentences. |
For generating my test data I switched to pyAIS, which is excellent for this purpose. The problem persists, some targets are missing. I don't think it is related to the input data. I also observed the following things.
Seems there are really some issues with AIS processing. |
For the removal of AIS data: |
Thanks! But these settings cannot be set in the webinterface? - Yes it can, in config section. good to know. It works, I just didn't wait the 20 minutes. |
from the log
|
Is it possible to get |
Increasing
Perfect! 😄 Looks like a rate limiter to me, so that nmea sentences are discarded if they come in too fast to prevent avnav from getting stalled and lagging behind. only the newest data in processed. right? Maybe it would be better to use a token bucket for rate limiting to allow for bursts instead of this hard seemingly pretty low limit of 10 entries? |
I poked around in the code with the debugger. Some thoughts on what I found.
|
|
Indeed. I think there is no way to handle out of sync clocks without knowing they are out of sync. How could you know that? But I would assume that the clocks are in sync since AIS is linked to GPS which provides the best time signal ever. It's probably safe to assume that all AIS transmitters use GPS time. Now you local clock has to be in sync with GPS time as well. avnav could maintain it's own clock internally (w/o modifying system time) in sync with GPS time. (This would also allow to speed up the clock in simulations for testing.) Anyways, are measure taken to sync the clocks of the avnav server and the client code in the browsers?
That is true. If there is no timestamp in the in the SK data, than you can only use arrival time in avnav. |
Is there a way to dynamically determine the max capacity of system and only drop messages if this limit is exceeded? What happens if writers generate backpressure now? |
My simulator is sending data every 1 second, this is not much. The point is, that AIS data cause bursts of data every 10s, but the average rate is about 7 sentences/s. |
|
Nice. - The more you think about it, the more interesting and complicated it gets, as always. |
I'm just thinking out loud here...
Image the following
👉 This is what causes this issue. IMHO this does not make sense to me because the system was idle, there was no pressure, it was not at its limit. There is no reason to discard any messages. - Currently it is assumed that messages trickle in one by one, not in bursts. But is this assumption justified? Hard to tell without a warning being displayed to the user. So, no one will notice without crawling through the logs and who does? It should work somehow like this
Now, of course it may happen that messages are added to buffer faster than the handler can process them causing a stalled system and delayed processing of the data. This must be prevented by discarding older or less important unprocessed messages. A warning should be displayed to tell the user about this. But this should only happen if the system hits its limit, not immediately on a burst which the system can handle. Who to do this? tbc |
Actually no, messages are discard even there is no backpressure. There is nothing in the implementation to communicate that backpressure upstream. |
Who to detect that a handler is not able to keep up with rate at wich messages arrive in the buffer?
I think in python this could be implemented nicely as a generator function yielding new messages and doing all of the sequence counter handling internally, such that the handler just gets the messages. I will try to build something... |
The concept now is rather simple: |
Hmmm maybe, have a look at my code when it's ready and discuss again. I don't think it makes sense to discard messages right away if a burst occurs, the messages should only be discarded if they cannot be processed fast enough. You don't need timestamps on each messages for this, just monitor the number of unprocessed messages. But putting timestamps on the messages when put into the buffer would be good anyways because these are the actual arrival timestamps. Now the timestamp is assigned too late during processing. |
|
Wait for the code, please, it shows the concept and is actually simple. All messages go into the buffer. If there are too many, it may overflow and old messages are lost, that's how it is and prevents huge bursts to kill the system. Now I want to drop messages per handler as currently implemented, but monitor the size of each handlers own pipeline and discard messages when the handler is not able to empty its own pipeline within a given time. This allows bursts to be handled with out dropping messages directly but also ensures that messages are processed within time X or dropped. |
Basically not really different except that you need a separate pipeline per handler. |
There is already a pipeline for each handler, each handler tracks its own sequence number, which is a pointer into the ring buffer, so this effectively is one pipeline per handler. There is no need to copy the messages into a separate list for each handler, the sequence number contains this information, but the handler should not bother with this counter, that could be encapsulated elsewhere. |
sure. |
…certain time (WIP)
Have a look, please: |
Hi Adam, |
Of course, I do not want and cannot force you to implement anything. How could I? I would like to convince you of my solution and/or learn in the discussion why it is bad. And even if my code ends up in the trash, I learnt something from it, this why I am digging into it. It's interesting do think about and play with that stuff. I am really impressed with what you've built here so far, basically as one-man-show. I recently discovered avnav and really like it. I would be happy, if I could contribute to the project. It's very interesting that people often are quite reluctant to accept changes. I can understand that to a certain extent and there are things that they know, which I don't, that are important to consider. This is where I am happy to learn something. But often their arguments are not really convincing me. Having a second avnav in a fork that diverges more and more doesn't make any sense and is just confusing, so it would be very nice to get this merged somehow. I would like to answer to your points.
I somehow can understand your feeling about it, maybe I am pushing this a bit too much and are writing too much, I know, don't bother. But what you say does not really convince me of my proposal being a bad solution or even a no-go. It does actually work. |
I'm always interested in discussions - can only make things better. I now have a first implementation that only has a couple of smaller changes. |
…ers, introduce an entry timestamp and a max message age
- no looping - force an empty queue
I would recommend to flush the queue completely. |
Lengthy discussion... And for the code elegance: |
Your solution is ok, it solves the current issue and is better than before. If a consumer is constantly too slow, it will not work well anyway, that's true. In either case messages have to be dropped. So, you're definitely loosing some data. Concerning your priorities of a stable API, I would prioritize differently, right. Maybe it's better not to change it. But I personally would change it since there are not hundreds of projects out there depending on it. This is your decision, of course. But using assertions really helps getting your code right with live tests (like index computation). They must not trigger in production anyway. If an assertion would trigger and you just remove it, because you don't want to have assertions in the code, then there will be some other exception or unwanted behavior that is hard to find. I don't get the point of not wanting assertions in the code, they reveal bugs, you fix them and on it goes. And error handling has to done anyways, with or without assertions.
This is actually not true! - Seems to me, you are not really familiar with Python's Your approach is somewhat similar now, but it only discards old messages (from time to time), it does not force the client to get the newest messages and leave an empty queue. After messages have been discarded, your implementation returns the next chunk of messages not older than specified, but these could actually "be sitting in the queue" for up to When using avnav as NMEA multiplexer congestion might occur (temporarily) at the outputs (serial or network). If that happens, your implementation could cause constantly delayed messages to be send to the outputs. If backpressure builts up, too old messages are discarded, but still messages are sent with a delay of up to
Having an arrival timestamp on the messages is good, if it is used downstream in the pipeline. The congestion problem could be solved without it.
Exactly as in your existing code. Have you seen what I wrote? ( All you do is to replace seq=0
while not should_stop():
seq,chunk = fetchFromHistory(seq)
for m in chunk: process(m) with (getting rid of for chunk in get_messages():
if should_stop(): break
for m in chunk: process(m) Now try-except blocks can be added to catch and log exceptions and keep it going, as it is currently done. The way the ringbuffer is implemented using a single int to track the message id is really clever. That does only work because Python as an infinitely sized int, it does not roll over. If it did, that might be a problem if you relied on the absolute value of the sequence counter, but differences would always be correct even across a rollover. But since multiple clients are reading from it and need to track their own progress, the sequence is returned with the data and passed in again. It works, but is confusing when reading the code and looks to me a bit like desperate workaround that now is "carved into stone" because you don't want to change the API. There is actually an easy way to make |
Thanks for the explanations. |
It's ok, you want to keep it your way.
This is quite an exaggeration looking at the example in my previous post. But nice, that avnav now can cope with burst of input data. |
I think this is fixed now with 58e850b. |
I don't know if it is a problem in my data but I experience the following. It works with SignalK but not with AvNav.
When I feed this AIS data into AvNav via a socket reader, it decodes well and all of the 10 targets are displayed correctly with their course vectors. For the class a targets the names are missing because there are no static data frames. This is ok so far.
If I now add static data frames (type 5) for the class a targets,
And even though the data is sent to AvNav for all targets at the same time (every 10s), the targets do not get updated simultaneously on the screen. Some lag behind way longer than 10s.
The data does decode well in SingalK, so when feeding the data into SK and then into AvNav it works as expected. The data does also decode as expected on https://www.maritec.co.za/aisvdmvdodecoding
AIS data generated with https://github.com/quantenschaum/mapping/blob/master/ship_sim.py
The text was updated successfully, but these errors were encountered: