Add Node time fallback for headerless messages (take 2)

[sgillen]

No description provided.

[greptile-apps]

Greptile Summary

This PR adds timestamp monitoring mode selection to handle headerless ROS 2 messages. It introduces three modes: header_with_nodetime_fallback (default - uses header timestamps for headered topics, node time for headerless), header_only (only monitors headered topics), and nodetime_only (always uses node time). The implementation detects message headers using a static type registry, stores header presence per topic, and conditionally enables node-time or message-time diagnostics based on the selected mode. The feature includes comprehensive unit and integration tests covering all mode combinations.

Confidence Score: 4/5

• Safe to merge with one minor style improvement recommended
• The implementation is well-tested with both C++ unit tests and Python integration tests covering all timestamp modes. The logic is sound and the feature properly handles edge cases like invalid mode strings. One minor style suggestion about adding a default case to the switch statement for defensive programming.
• No files require special attention - all changes are well-structured and tested

Important Files Changed

Filename	Overview
greenwave_monitor/include/greenwave_diagnostics.hpp	Added TimestampMonitorMode enum, node time drop tracking (prev_drop_node_ts_), and extended setExpectedDt to conditionally enable node/msg checks
greenwave_monitor/src/greenwave_monitor.cpp	Implemented parameter parsing, mode resolution logic in resolve_timestamp_checks, and integrated header detection with diagnostics configuration
greenwave_monitor/test/test_greenwave_diagnostics.cpp	Added comprehensive unit tests for all three timestamp monitor modes, updated existing test assertions to handle composite error messages
greenwave_monitor/test/test_greenwave_monitor.py	Added integration tests for timestamp mode parameter parsing, fallback behavior, and invalid mode handling

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Start[Topic monitoring request] --> ParseMode[Parse timestamp_monitor_mode parameter]
    ParseMode --> CheckHeader{Topic has header?}
    
    CheckHeader -->|Yes - Header available| HeaderedPath[Headered topic path]
    CheckHeader -->|No - Headerless| HeaderlessPath[Headerless topic path]
    
    HeaderedPath --> ModeCheck1{Monitor mode?}
    ModeCheck1 -->|header_with_nodetime_fallback| HF_H[Enable: msg checks only]
    ModeCheck1 -->|header_only| HO_H[Enable: msg checks only]
    ModeCheck1 -->|nodetime_only| NO_H[Enable: node checks only]
    
    HeaderlessPath --> ModeCheck2{Monitor mode?}
    ModeCheck2 -->|header_with_nodetime_fallback| HF_HL[Enable: node checks only - FALLBACK]
    ModeCheck2 -->|header_only| HO_HL[Disable: both checks]
    ModeCheck2 -->|nodetime_only| NO_HL[Enable: node checks only]
    
    HF_H --> Monitor[Monitor topic with config]
    HO_H --> Monitor
    NO_H --> Monitor
    HF_HL --> Monitor
    HO_HL --> Monitor
    NO_HL --> Monitor
    
    Monitor --> Diagnostics[Publish diagnostics]

Loading

_{Last reviewed commit: 2c0d1d8}

[greptile-apps]

_{8 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Missing default case in switch statement

The switch covers all enum values but lacks a default case. Add default: return std::make_pair(false, false); for defensive programming and to silence potential compiler warnings.

Suggested change

	std::pair<bool, bool> resolve_timestamp_checks(
	greenwave_diagnostics::TimestampMonitorMode mode, bool topic_has_header)
	{
	switch (mode) {
	case greenwave_diagnostics::TimestampMonitorMode::kHeaderWithNodetimeFallback:
	return topic_has_header ? std::make_pair(false, true) : std::make_pair(true, false);
	case greenwave_diagnostics::TimestampMonitorMode::kHeaderOnly:
	return topic_has_header ? std::make_pair(false, true) : std::make_pair(false, false);
	case greenwave_diagnostics::TimestampMonitorMode::kNodetimeOnly:
	return std::make_pair(true, false);
	}
	return std::make_pair(false, false);
	}
	switch (mode) {
	case greenwave_diagnostics::TimestampMonitorMode::kHeaderWithNodetimeFallback:
	return topic_has_header ? std::make_pair(false, true) : std::make_pair(true, false);
	case greenwave_diagnostics::TimestampMonitorMode::kHeaderOnly:
	return topic_has_header ? std::make_pair(false, true) : std::make_pair(false, false);
	case greenwave_diagnostics::TimestampMonitorMode::kNodetimeOnly:
	return std::make_pair(true, false);
	default:
	return std::make_pair(false, false);
	}

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}