Support speed and NMEA heading from Simrad .raw files#1703
Support speed and NMEA heading from Simrad .raw files#1703gavinmacaulay wants to merge 15 commits into
Conversation
- Add speed over ground to the EchoData object (obtained from NMEA RMC and VTG messages), - Add heading data to the EchoData object for EK60 files and EK80 files (if heading is not available from MRU datagrams). Obtained from NMEA HDT messages, - Add tests for the new code. The test for EK80 heading from HDT uses a .raw file that needs to be added to the test assets (will be attached to this PR). Note: an existing comment in `convert/set_groups_base.py` for a function this PR modifies says "move this to be part of parser as it is not a 'set' operation". This has not been done as it would get muddy the changes in this PR.
for more information, see https://pre-commit.ci
|
The unit tests look to all be failing due to not finding the extra .raw file that needs to be added to the test assets. |
|
Hi @gavinmacaulay, thx for the PR! i'll had the data first thing tomorrow and will then re-trigger the set of test! cheers! |
- Resolved duplicated timestamps when extracting from NMEA - Upped number on time dimensions for heading and sog as they interfered with other time dimension names - Add support for time dimension names beyond `time9` - Modified some tests to adjust to changed time dimension names Notes: - The change to convert/set_groups_base.py where reordering of dimensions is removed is tentative. I'm not sure if the reordering of dimensions is necessary or just nice to have. - Some tests hard-coded the time dimension names which makes them a bit fragile. This also suggests that users may have done the same in their code and adding new time dimensions of `time10` and `time11` may cause problems - My local testing fails on the tests that get data via minio so there may be real test failures in there...
for more information, see https://pre-commit.ci
|
@LOCEANlloydizard - just pushed a new set of changes that should address most of the test failures not related to that new data file |
add data for echostack-org#1703
|
@gavinmacaulay i've added the new test data file as a separate release asset (ek80_heading.zip) in #1704 So the PR needs small changes:
let me know if something doesnt work! |
|
@gavinmacaulay I've merged #1704 into main, so you can(/need) pull the latest changes! cheers! |
|
@LOCEANlloydizard Thanks for adding the extra raw file. Tests under Windows now all pass but those under Ubuntu don't. It looks to be because when ek80_heading.zip is unzipped the raw file ends up in I suspect (am not 100% sure) that the directory structure in the .zip file needs to be |
for more information, see https://pre-commit.ci
|
Similar problem still:) On windows the raw file ends up in ek80_heading/ but on ubuntu it is in ek80_heading/heading/ |
|
@gavinmacaulay i've updated the ZIP file to match the actual test path (ek80_heading_path / "D20260613-T230914.raw")! Yes, I ran into this issue before between Windows and Ubuntu... and I think the flattening function may need a bit more investigation. In any case, it passes CI now! By the way, is this PR ready for review, or is it still a draft? Thanks! |
|
@LOCEANlloydizard Ready for review now (was just being careful and waiting for all checks to properly pass). |
|
did not review the code, just putting this as a note for later: how are duplicate timestamps handled for these new variables? e.g. #1627 |
|
Duplicate timestamps are removed in I've just now noticed #1478 (comment) and note that removing duplicate timestamps for lat/lon resolves that problem in a different, perhaps unwanted, way. And a similar problem may occur with the speed, which currently reads from RMC and VTG NMEA messages. |
This PR addresses issue #1346
ek80/headingdirectory and is used bytest_convert_ek80::test_parse_NMEA_heading.Notes:
convert/set_groups_base.pyfor a function this PR modifies says "move this to be part of parser as it is not a 'set' operation". This has not been done as it would muddy the changes in this PR,