OceanDataTools / openrvdas

An open source data acquisition system designed for use on research vessels and other scientific installations
http://openrvdas.org
Other
39 stars 20 forks source link

Revamp UDPReader #352

Closed veggiemike closed 10 months ago

veggiemike commented 10 months ago

Hey, Pablo! Happy weekend! Don't read this until Monday, unless you really want to. :-) This batch of commits makes changes similar to the ones in recent UDPWriter and TCPWriter pull requests. Improvements have been made to name resolution, text and raw/binary input handling, multicast and broadcast handling, optimized buffer sizes, configurable socket options, removal of TCP-centric conditional code, bet test coverage, etc. This will be followed up with a "Replace NetworkReader with TCPReader" pull request once I've finished testing that code.

davidpablocohn commented 10 months ago

Perfect - wish I'd thought of that!

On Mon, Oct 30, 2023 at 11:54 AM Michael D Labriola < @.***> wrote:

@.**** commented on this pull request.

In logger/readers/udp_reader.py https://github.com/OceanDataTools/openrvdas/pull/352#discussion_r1376678727 :

 ############################

-

  • def init(self, port, source='', eol=None,
  • read_buffer_size=READ_BUFFER_SIZE,
  • def init(self, interface, port, mc_group=None,

Ah, yup, I see your point there.

I don't like the idea of a real default port. I think our best compromise is to add port=None to the parameter list, then throw an exception if port wasn't specified. So in Python's eyes, it's an optional kwarg, but in reality we make it required. Then we can maintain the behavior in the .yml you just described and have a more natural dest,port pairing consistent across all the networking Readers/Writers.

Or, we reverse port/interface in UDPReader and TCPReader (once finished) so it's more Python-correct.

I'm gonna go implement the port=None workaround and see if I hate it. Thoughts?

— Reply to this email directly, view it on GitHub https://github.com/OceanDataTools/openrvdas/pull/352#discussion_r1376678727, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFO7V3STJK2ESBGLLZ2MXYLYB7ZXLAVCNFSM6AAAAAA6USF7ZOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMBUHA3DSMRYGQ . You are receiving this because you commented.Message ID: @.***>