dsuarezv / mavlink.net

A better MavLink object generation for C#. Richer message classes are generated from the object definitions.
38 stars 35 forks source link

BlockedCircularStream problem #2

Closed codenotes closed 10 years ago

codenotes commented 10 years ago

Hi,

Been trying to leverage your library to read messages over UDP...specifically the UDPTransport.

I recieve a few messages, but then very quickly I get an Exception, the one shown below that is internal to mavlinknet.dll:

               if (len2 > mReadPosition)
                {
                    throw new InternalBufferOverflowException("Data is being overwritten without being read. May need to increase capacity.");
                }

I am not really sure what to do or how to adjust for this error. How does one increase capacity? How does one "read" the data off the queue faster.

Any advice? Your library is precisely what I need if I can only get this working...

codenotes commented 10 years ago

Debugged a little more. Turns out that there are some mavlink datagrams that are larger than the 4096 that the circular buffer allows for. So I changed:

public const int DefaultCircularBufferSize = 8192;

And while that did remove the exception, I no longer get any message notifications. So i wonder if there is a hard-coded preference for a 4096byte size buffer elsewhere in the mavlinknet.dll code...

Appreciate any guidance...

Best, Gregory

dsuarezv commented 10 years ago

The circular buffer should be a few times larger than the largest message you are going to receive on the wire to prevent buffer overflows. There is no hardcoded size other than the constant you mention.

About not receiving notifications, try to subscribe to the PackectDiscarded event of the MavLinkAsyncWalker class, or set a breakpoint on MavLinkAsyncWalker.PacketProcessingWorker(...) in the NotifyPacketDiscarded(packet) call, to check if your packet is being discarded. It so, it means that the parser failed to handle it at some point (maybe CRC check).

Is your type of packet correctly generated from the XML definitions?

Cheers!

David

codenotes commented 10 years ago

Looking more into it, a serial to udp translator I have going (the source is a serial device) is putting some junk on the wire and that is what mavlink is having exception to. I wrote my own serial-to-udp and I am not seeing the buffer error.

Only issue now is that I need to get this working with .net 3.5, so am struggling with the ConcurrentQueue class not existing earlier (the UDP transport is based around it.) Can't use 4.x sadly as I am developing a manvlink library for Unity3d, and that doesn't go much higher than 3.5. But that's my issue to work out...

Thank you for the prompt response earlier.

dsuarezv commented 10 years ago

About the concurrent queue, you should be fine replacing it with a regular queue and locking around any access to it:

lock (mylockobject) { queue.TryDequeue(...) }

use different lock objects for the two queues.

codenotes commented 10 years ago

Yeah, actually...I got the "Real" concurrentqueue source from mono, and just brought it into the project...surprisingly, it didn't have too many dependencies and worked. What I am struggling with now is a pretty big memory leak somewhere...I watch the memory climb 10mb, -20mb...about a MB a minute or so while it is running. Ever see anything like that?

codenotes commented 10 years ago

One other question, if I can ask...(and btw, really terrific library and well crafted)

In my scenario, I can't really have any callbacks. Unity will be asking for packets every frame (it is a polling architecture), so I am thinking I should eliminate the PacketProcessingWorker thread, and instead just reach into the circularstream myself (just as the worker thread was doing) and report that upwards when polled.

Does that sound like the appropriate approach? I will confess I don't fully understand all the buffer work that is going on in MavWalker and CircularBuffer, but it seems like that is the right way to go...

dsuarezv commented 10 years ago

Uhm, maybe it doesn't work as you expect. The BlockingCircularStream is designed to block until the expected number of bytes is available. So the SyncStream and MavlinkPacket.Deserialize methods actually are waiting until the full packet is received (on separate threads).

I guess your polling Update method should return as quickly as possible, but the circular stream will block until the requested input is filled in. From the Read implementation of the circular buffer:

 /// The read operation will return 'count' bytes from the stream if they
 /// are available. Otherwise, this method will block until enough bytes
 /// are written through the Write() method from another thread.

In this case I think it is better to skip the circular buffer altogether. For a polling implementation, I would suggest looking at the Socket.Select method. Accumulate enough bytes to parse a packet in a buffer and send them to the packet deserialize code.

codenotes commented 10 years ago

Thanks for the thoughts, this helps. Any thinking on the memory leak? I am finding the library grows quite a bit as it runs in terms of memory utilization. Over 1mb a minute about.

dsuarezv commented 10 years ago

I'm not aware of any leaks running under .net. Are you running under Mono with the Boehm garbage collector?