Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vh 1216 fix spat plugin segfault simulation #621

Merged
merged 31 commits into from
Jul 31, 2024

Conversation

paulbourelly999
Copy link
Contributor

@paulbourelly999 paulbourelly999 commented Jul 8, 2024

PR Details

Description

This pull request updates the SPAT plugin to fix a number of issues with the legacy implementation.

  • Overly complex legacy code in SPaT Plugin
  • Reduced duplication using existing classes for SNMP Client and UDP Server. Previous implementation created custom SNMP Client and UDP server.
  • Reduced use of raw pointers to mitigate the possibility of memory leaks.
  • Improved documentation for SPaT plugin.
  • Added unit testing
  • Added functionality to consume J2735 UPER HEX SPaT from signal controller.

Additionally to help provide these improvements in the SPaT plugin, several improvements were made in the TMX Utils functionality.

  • Fix SNMP Client to have working SNMP Set implementation (allows reused of existing client instead of standalone SNMP client implementation for SPaT plugin)
  • Improvement PluginClientClockAware base class to automatically subscribe to TimeSync messages and getter for clock to wait for clock initialization. These improvements removed all simulation specific implementation from inheriting classes including the SpatPlugin

Related Issue

VH-1216

Motivation and Context

Fix SPaT plugin for simulation and improve overall usability and maintainability.

How Has This Been Tested?

Tested using virtual signal controller for both binary and hex spat payloads.
Also tested in sim mode using the following steps

  1. Deploy Virtual Signal Controller.
  2. Deploy V2X-Hub in SIM MODE using Dev Container deployment
  3. Run src/v2i-hub/CDASimAdapter/scripts/send_timestep_udp.py script to send periodic time sync messages.
  4. Enable SPaT Plugin

Types of changes

  • Defect fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that cause existing functionality to change)

Checklist:

  • I have added any new packages to the sonar-scanner.properties file
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
    V2XHUB Contributing Guide
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@paulbourelly999 paulbourelly999 marked this pull request as ready for review July 18, 2024 16:01
@paulbourelly999 paulbourelly999 marked this pull request as draft July 18, 2024 17:51
@paulbourelly999 paulbourelly999 marked this pull request as ready for review July 22, 2024 13:17
@paulbourelly999 paulbourelly999 requested review from DokurOmkar and removed request for jwillmartin July 22, 2024 19:11
Copy link
Contributor

@DokurOmkar DokurOmkar left a comment

Choose a reason for hiding this comment

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

There are some places where more error handling can be added especially while working with the clock. Suggest adding those in there.

Also, variable naming can be fixed to be consistent across.

src/v2i-hub/SpatPlugin/src/SpatPlugin.cpp Show resolved Hide resolved
src/tmx/Messages/test/Main.cpp Outdated Show resolved Hide resolved
src/tmx/TmxUtils/src/PluginClientClockAware.h Show resolved Hide resolved
src/tmx/TmxUtils/src/SNMPClient.cpp Show resolved Hide resolved
src/tmx/TmxUtils/src/SNMPClient.cpp Show resolved Hide resolved
src/v2i-hub/CDASimAdapter/scripts/send_timestep_udp.py Outdated Show resolved Hide resolved
src/v2i-hub/SpatPlugin/CMakeLists.txt Show resolved Hide resolved
src/v2i-hub/SpatPlugin/README.md Outdated Show resolved Hide resolved
src/v2i-hub/SpatPlugin/src/NTCIP1202OIDs.h Outdated Show resolved Hide resolved
src/v2i-hub/SpatPlugin/src/SignalControllerConnection.cpp Outdated Show resolved Hide resolved
src/v2i-hub/SpatPlugin/src/SignalControllerConnection.cpp Outdated Show resolved Hide resolved
src/v2i-hub/SpatPlugin/src/SignalControllerConnection.cpp Outdated Show resolved Hide resolved
src/v2i-hub/SpatPlugin/src/SignalControllerConnection.cpp Outdated Show resolved Hide resolved

## Introduction

The SPaT Plugin is responsible for receiving information from the Traffic Signal Controller (TSC or SC) necessary for broadcasting Signal Phase and Timing (SPaT) messages. This includes querying any SNMP objects to determine TSC state and listen for any broadcast SPaT information from the TSC.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this received information used to populate self-defined Ntcip1202 https://github.com/usdot-fhwa-OPS/V2X-Hub/blob/develop/src/v2i-hub/SpatPlugin/src/NTCIP1202.h? Can we add additional details about the received information

src/v2i-hub/SpatPlugin/src/SpatPlugin.h Show resolved Hide resolved
src/v2i-hub/SpatPlugin/src/NTCIP1202.h Outdated Show resolved Hide resolved
src/v2i-hub/SpatPlugin/test/TestNTCIP1202.cpp Show resolved Hide resolved
if (state == IvpPluginState_registered) {
UpdateConfigSettings();
SpatPlugin::SpatPlugin(const std::string &name) :PluginClientClockAware(name) {
spatReceiverThread = std::make_unique<tmx::utils::ThreadTimer>(std::chrono::milliseconds(5));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason we choose 5 milliseconds?

Copy link

sonarcloud bot commented Jul 30, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
37.87% Line Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@paulbourelly999 paulbourelly999 merged commit 2303a49 into develop Jul 31, 2024
2 of 3 checks passed
@paulbourelly999 paulbourelly999 deleted the vh-1216-fix-spat-plugin-segfault-simulation branch July 31, 2024 13:12
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.

3 participants