search

dgiardini / rtl-ais

A simple AIS tuner and generic dual-frequency FM demodulator
Other
264 stars 90 forks source link

New feature: `-k` flag (keep streaming new data to TCP connections) #34

Open mik3y opened 3 years ago

mik3y commented 3 years ago

This change set adds the -k flag, which affects the TCP listener. When -k is given, the TCP socket will continue to publish newly-decoded messages to any connected clients.

I've tried to keep unnecessary diffs to a minimum. Here's a summary of the major parts of the change:

typedef struct t_sockIo {...}

The struct which tracks open TCP clients is extended to add int msgpipe[2]. These will store two file descriptors, as created by pipe(2), for the decoder thread to publish to the connection thread. This pipe is created when a new client connects, and destroyed upon disconnect.

add_nmea_ais_message(..)

When a new message is decoded, and the global _tcp_stream_forever option is set, this method is extended to publish the message to all connected clients. It does do by write(2)ing to the msgpipe fd.

handle_remote_close(..)

This method continues to service remote TCP clients. After publishing buffered messages (existing functionality which is unchanged), it waits on a blocking pselect(2) loop for one of two things to happen:

When -k is not specified, no data is ever published to msgpipe; so the existing behavior remains intact.

Other/unrelated changes

There are two other commits here which address minor/unrelated things I ran into. It might be easier to review by looking at commits one at a time, which should be coherent.

Closes #33.

dgiardini commented 3 years ago

Can't compile on Windows, would be nice to to keep it portable. The problem is with the pselect() function, it's not available and we can't select() on a pipe on this OS. Since we don't need IPC, just communication between threads, maybe we can find another approach to send the data. Let me know if you have something in mind.

On Wed, Feb 3, 2021 at 10:17 PM mike w notifications@github.com wrote:

This change set adds the -k flag, which affects the TCP listener. When -k is given, the TCP socket will continue to publish newly-decoded messages to any connected clients.

I've tried to keep unnecessary diffs to a minimum. Here's a summary of the major parts of the change: typedef struct t_sockIo {...}

The struct which tracks open TCP clients is extended to add int msgpipe[2]. These will store two file descriptors, as created by pipe(2) https://man7.org/linux/man-pages/man2/pipe.2.html, for the decoder thread to publish to the connection thread. This pipe is created when a new client connects, and destroyed upon disconnect. add_nmea_ais_message(..)

When a new message is decoded, and the global _tcp_stream_forever option is set, this method is extended to publish the message to all connected clients. It does do by write(2)ing https://man7.org/linux/man-pages/man2/write.2.html to the msgpipe fd. handle_remote_close(..)

This method continues to service remote TCP clients. After publishing buffered messages (existing functionality which is unchanged), it waits on a blocking pselect(2) https://linux.die.net/man/2/pselect loop for one of two things to happen:

  • New data is available on the read fd of msgpipe. It writes it out to the tcp socket.
  • New data arrives from the tcp socket. By previous convention, this causes the server to close the socket.
    • It's not clear to me if that behavior remains necessary here; i.e., we could ignore reads and let the client close when it wants.

When -k is not specified, no data is ever published to msgpipe; so the existing behavior remains intact. Other/unrelated changes

There are two other commits here which address minor/unrelated things I ran into. It might be easier to review by looking at commits one at a time, which should be coherent.

You can view, comment on, or merge this pull request online at:

https://github.com/dgiardini/rtl-ais/pull/34 Commit Summary

  • Fix build on MacOS X (Darwin)
  • Fix some self-assignment warning [-Wself-assign]
  • New feature: Stream new messages in TCP mode

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dgiardini/rtl-ais/pull/34, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADBJPT6VBPDILTGRB2FC4QTS5HYTDANCNFSM4XB4NGIQ .

mik3y commented 3 years ago

Hmm, that's a problem indeed. As an alternative, could write() to all connected clients synchronous with decode. Don't see great need for a thread per connection, so would do away with that too.

I don't have a windows environment to build/test with so I'll probably have to leave this to others.

rvt commented 3 years ago

Could also enable the -k switch for *nix for the moment? At least that will help a set of users that want this.

mik3y commented 3 years ago

@rvt if you want to use this right now, I have a project that incorporates my fork into a docker build. Should be usable: https://github.com/mik3y/rtl-ais-docker

I'm happy to take another swing at this change, but it'll take me at least a few days to find the time. As to approach, I'm still inclined to remove the unnecessary thread-per-connection & eliminate the internal signaling altogether.

dgiardini commented 3 years ago

Mike, would be great if you can do those changes. If you think that you need a considerable time (a month or so); I could add some conditionals so it can compile in all systems and keep the -k feature for the unix-like.

On Fri, Oct 8, 2021 at 2:39 PM mike w @.***> wrote:

@rvt https://github.com/rvt if you want to use this right now, I have a project that incorporates my fork into a docker build. Should be usable: https://github.com/mik3y/rtl-ais-docker

I'm happy to take another swing at this change, but it'll take me at least a few days to find the time. As to approach, I'm still inclined to remove the unnecessary thread-per-connection & eliminate the internal signaling altogether.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dgiardini/rtl-ais/pull/34#issuecomment-938946583, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADBJPT55KIFYDNYDKR73GITUF4UFDANCNFSM4XB4NGIQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

This message represents the official view of the voices in my head

rvt commented 3 years ago

Hey Mike,

If I use your branch (https://github.com/mik3y/rtl-ais.git) will I get the version that keeps streaming with new connections?

I am working on a project where I cannot us the docker container but does the build, https://github.com/rvt/stratux/tree/stratux-ais and I need to have some guarantee that if a connection drops it will be picked up again.

mik3y commented 3 years ago

Hi @rvt: Yes, my docker project points to my fork, here are the pertinent lines.

I'm not planning to touch that project again until resolving this issue (i.e. getting something officially merged), so feel free to use it directly or fork it.

rvt commented 3 years ago

ok great,

Unfortunately I do not have a windows machine to test and modify code so I cannot help here... I will use your branch in the mean time.