search

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

Support raw/binary output with UDPWriter #345

Closed veggiemike closed 10 months ago

veggiemike commented 10 months ago

Ahoy! I ran into a couple issues this last cruise aboard the E/V Nautilus and decided to get my hands dirty and fix them. My primary issue was that we couldn't read raw binary data with a NetworkReader and then send it unmodified to a couple different systems using UDPWriter instances. NetworkReader handles raw/binary data fine by setting encoding=None, but UDPWriter was blindly encoding it into UTF-8, which obviously garbles it all up.

I tried to keep this branch focused on just this issue, but couldn't help but fix a couple other problems I bumped into along the way. I actually have a whole 2nd branch full of SerialWriter, NetworkWriter, and testsuite fixes to send along after this, but figured I'd start small(ish). We're planning on using OpenRVDAS more and more on the Nautilus, so I'm going to keep on find-and-fixing while I'm out here.

All the tests that passed before still pass (there are some SerialWriter errors already present that I've got fixed in my next branch).

davidpablocohn commented 10 months ago

Hi Mike - this looks great, thank you!

I'll dive into reviewing shortly, but may I ask that you refile the pull request to pull it into the OpenRVDAS dev branch? I try to aggregate changes there, then push everything out from dev-> master at once.

On Tue, Oct 17, 2023 at 12:56 PM Michael D Labriola < @.***> wrote:

Ahoy! I ran into a couple issues this last cruise aboard the E/V Nautilus and decided to get my hands dirty and fix them. My primary issue was that we couldn't read raw binary data with a NetworkReader and then send it unmodified to a couple different systems using UDPWriter instances. NetworkReader handles raw/binary data fine by setting encoding=None, but UDPWriter was blindly encoding it into UTF-8, which obviously garbles it all up.

I tried to keep this branch focused on just this issue, but couldn't help but fix a couple other problems I bumped into along the way. I actually have a whole 2nd branch full of SerialWriter, NetworkWriter, and testsuite fixes to send along after this, but figured I'd start small(ish). We're planning on using OpenRVDAS more and more on the Nautilus, so I'm going to keep on find-and-fixing while I'm out here.

All the tests that passed before still pass (there are some SerialWriter errors already present that I've got fixed in my next branch).

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

https://github.com/OceanDataTools/openrvdas/pull/345 Commit Summary

File Changes

(3 files https://github.com/OceanDataTools/openrvdas/pull/345/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/OceanDataTools/openrvdas/pull/345, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFO7V3VWSRQIU4ENM3QDCZTX73PHXAVCNFSM6AAAAAA6EMJBXSVHI2DSMVQWIX3LMV43ASLTON2WKOZRHE2DQMJUHA2DSNI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

veggiemike commented 10 months ago

No problem, Pablo! I've changed the pull target to OceanDataTools:dev. Thanks!

davidpablocohn commented 10 months ago

Excellent - Thanks! Getting right on this. (And yeah, the feature/bug fix makes perfect sense - thank you for diving in on that!)

On Tue, Oct 17, 2023 at 1:34 PM Michael D Labriola @.***> wrote:

No problem, Pablo! I've changed the pull target to OceanDataTools:dev. Thanks!

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

veggiemike commented 10 months ago

Thanks, Pablo!