Skip to content

Conversation

@ANAVHEOBA
Copy link

Summary

This PR addresses three open issues related to node registration, DDS subscription handling, and scenario processing.

Changes

#391 - Node registration wrong IP

  • Fixed find_node_by_simple_key() to filter out invalid IPs (0.0.0.0, 127.0.0.1, empty)
  • Now iterates through all nodes to find first valid IP

#389 - Filtergateway condition bug

  • Skip DDS subscription when scenario has no conditions or empty topic name
  • Prevents dummy DDS data from being published

#386 - Scenario processing only last scenario

  • Changed apply() to return all scenarios from YAML, not just the last one
  • All scenarios in a multi-document YAML are now sent to FilterGateway

Test Plan

  • Verify node registration returns valid IP addresses
  • Confirm scenarios without conditions don't trigger DDS subscriptions
  • Test multi-scenario YAML files are fully processed

- Modified find_node_by_simple_key() to skip invalid IPs (0.0.0.0, 127.0.0.1, empty)
- Iterates through all nodes to find first valid IP instead of just returning first found
- Added new find_node_by_target_ip() function for specific IP lookups
- Prevents node registration from returning wrong/unusable IP addresses
…-pullpiri#389]

- Check if scenario has valid conditions before subscribing to DDS topic
- Skip subscription when topic name is empty to prevent dummy data
- Added logging for skipped subscriptions for debugging
- Fixes issue where empty conditions caused unwanted DDS data publishing
…i#386]

- Changed apply() to return Vec<String> instead of single String
- Collects all scenarios from multi-document YAML, not just the last one
- Updated apply_artifact() to send all scenarios to FilterGateway
- Updated tests to handle new return type
- Fixes issue where only last scenario in YAML file was being processed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deeply appreciate your contribution.

This section inserts a random node when the package fails to find a node to run on. As a result, the code shouldn't be executed, and it would be better to fundamentally change the part before this point.

|| ip_address == "127.0.0.1"
{
println!("Skipping invalid IP: {}", ip_address);
continue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes, IP can be 127.0.0.1 or 0.0.0.0.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice bug fix

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good.
However, no decision has yet been made on whether to allow multi-scenarios.

@daeyoung-jeong-lge
Copy link
Contributor

@ANAVHEOBA Thanks for the contribution!! Unfortunately, you need Eclipse FDN for PR to be merged. So, you need to join Eclipse foundation and submit some materials for getting ECA. If you are still considering the contribution, please get Eclipse FDN/ECA. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants