-
Notifications
You must be signed in to change notification settings - Fork 89
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
New feature: -k
flag (keep streaming new data to TCP connections)
#34
base: master
Are you sure you want to change the base?
Conversation
Uses pkg-config to locate libraries and headers where homebrew puts them.
When giving the `-T` flag, historical behavior is that the previously received, buffered messages are streamed to the client, and then no more. The socket is kept open until either the client closes it, or until the client sends any data, at which point the client is disconnected. When the `-k` flag is given in addition to the `-T` flag, an alternative behavior is now implemented: As new messages are decoded, they are sent to any attached clients in realtime. The client does not need to disconnect to continue receiving updates. Closes dgiardini#33
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 ***@***.***> 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:
#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
- *M* Makefile
<https://github.com/dgiardini/rtl-ais/pull/34/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52>
(44)
- *M* aisdecoder/aisdecoder.c
<https://github.com/dgiardini/rtl-ais/pull/34/files#diff-cd0289f61c8bac9742087d98cadea7050f3e49caedb456ee50e027e116cebfc5>
(4)
- *M* aisdecoder/aisdecoder.h
<https://github.com/dgiardini/rtl-ais/pull/34/files#diff-0f48079dfb402055ac801d448362f78617bde6f71d8be1855a5d20a8c6fb715f>
(2)
- *M* aisdecoder/lib/protodec.c
<https://github.com/dgiardini/rtl-ais/pull/34/files#diff-010dd813073904b075aa78fa8932760c700955597c1c695373e0ff055ca4e090>
(2)
- *M* main.c
<https://github.com/dgiardini/rtl-ais/pull/34/files#diff-a0cb465674c1b01a07d361f25a0ef2b0214b7dfe9412b7777f89add956da10ec>
(20)
- *M* rtl_ais.c
<https://github.com/dgiardini/rtl-ais/pull/34/files#diff-8a885b5d169381e75cf5dfa5e65edc0a7152481484ed9f709de0c81b0e9fb8e5>
(5)
- *M* rtl_ais.h
<https://github.com/dgiardini/rtl-ais/pull/34/files#diff-71c2b5146b56da3e731912d9197c719b052d6c8460f5fb3b7b6db1c89c772531>
(2)
- *M* tcp_listener/tcp_listener.c
<https://github.com/dgiardini/rtl-ais/pull/34/files#diff-659acb69fe5d40059c26afd900edc498c230e41f8363441de1a3eff6df0834ae>
(140)
- *M* tcp_listener/tcp_listener.h
<https://github.com/dgiardini/rtl-ais/pull/34/files#diff-d7ea14c01137caf91aa98718477f13b0e612d85ff09b3059c4d576a85b4d70b5>
(2)
Patch Links:
- https://github.com/dgiardini/rtl-ais/pull/34.patch
- https://github.com/dgiardini/rtl-ais/pull/34.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#34>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADBJPT6VBPDILTGRB2FC4QTS5HYTDANCNFSM4XB4NGIQ>
.
|
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. |
Could also enable the -k switch for *nix for the moment? At least that will help a set of users that want this. |
@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. |
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
<#34 (comment)>, 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
|
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. |
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. |
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. |
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 bypipe(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 bywrite(2)
ing to themsgpipe
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:msgpipe
. It writes it out to the tcp socket.When
-k
is not specified, no data is ever published tomsgpipe
; 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.