Conversation
| # MROV packet format: | ||
| # MDS YYYY/MM/DD HH:MM:SS.SSS Lat Lon Z Depth Alt SOG COG Northing Easting Roll Pitch Heading |
There was a problem hiding this comment.
One thing I would like to have for each message type is a fully populated example, not only the field labels. This way we always have a documented test case at hand if we need it (though it would be smart to like, actually have test cases).
| data = packet.decode().rstrip('\n').split(' ') | ||
|
|
||
| if not data or data[0] != 'MDS': | ||
| logger.debug('MDS line header missing') |
There was a problem hiding this comment.
Sometimes we have multiplexed streams with different message types coming in on the same port, so I wouldn't log or it could get very noisy.
There was a problem hiding this comment.
That's just a debug message, only logged if you set --log-level DEBUG
| # Extract MROV fields and map to standard nav data order | ||
| # MROV: MDS timestamp Lat Lon Z Depth Alt SOG COG Northing Easting Roll Pitch Heading | ||
| field_values = [ | ||
| data[3], # latitude -> latitude | ||
| data[4], # longitude -> longitude | ||
| data[10], # northing -> local_x | ||
| data[11], # easting -> local_y | ||
| data[12], # roll -> roll | ||
| data[13], # pitch -> pitch | ||
| data[14], # heading -> heading | ||
| data[6], # depth -> depth | ||
| data[7], # altitude -> altitude | ||
| ] |
There was a problem hiding this comment.
I am not sure this is necessary (and it's a little confusing). Yes, the data_array will come out in a different order, but it's a key-value mapping in a trench coat.
If there's a display issue in Sealog, we should refactor the nav display to force the ordering. But I thought we might already do that.
There was a problem hiding this comment.
I was mostly trying to keep the data identical to other products because I have no idea what kinds of downstream scripts might be out there using indices instead of keys.
There was a problem hiding this comment.
I'm pretty confident there are none. If there were, I would be happy to break them so we can make them more robust (there is clearly one correct way to pull an entry out of a list of (key, value) pairs). I think there's a risk of ossification we need to resist or the system crosses a critical threshold of fragility.
| logger.debug(f'Found aux data for event {event_id}: timestamp={aux_data_ts}, data_source={aux_data.get("data_source", "unknown")}') | ||
| logger.debug(f'Aux data details: {aux_data}') | ||
| except ValueError as e: | ||
| logger.warning(f'No aux data available for event {event["message"]["id"]}: {e}') | ||
| logger.debug(f'Cache state - size: {len(AUX_DATA_CACHE)}, timestamps: {[entry.timestamp for entry in AUX_DATA_CACHE[:5]]}...') |
There was a problem hiding this comment.
Some very long lines used for logging here and below, could those be wrapped please?
| if abs((event_ts - aux_data_ts).total_seconds()) > ARGS.max_age: | ||
| logger.info('Ignoring event older than maximum age') | ||
| if time_diff > ARGS.max_age: | ||
| logger.info(f'Ignoring event {event_id} - aux data too old (age: {time_diff}s > max: {ARGS.max_age}s)') |
There was a problem hiding this comment.
Probably want {:.1f} or {:.0f} when printing time_diff due to precision.
Might be clearer: Not adding stale aux data to {event_id} (age: {time_diff:.1f}s > max: {ARGS.max_age}s)
| else: | ||
| logger.warning(f'API request failed for event {event_id}: status={response.status_code}, response={response.text}') | ||
| except Exception as e: | ||
| logger.error(f'Exception occurred while posting aux data for event {event_id}: {e}') |
There was a problem hiding this comment.
Please use logger.exception() in except blocks. But actually here if we are Python 3.11 (and we ought to be by now), we can do
except Exception as e:
e.add_note(f'Exception occurred while posting aux data for event {event_id}')
raiseThere was a problem hiding this comment.
Unfortunately... 3.9
ndsf-sealog-scripts/Dockerfile
Line 1 in 532cbc5
Adding support for UDP streams coming from the MROV project. Enhanced logging for easier debugging.