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

Fix ped plugin #620

Merged
merged 12 commits into from
Aug 16, 2024
Merged

Fix ped plugin #620

merged 12 commits into from
Aug 16, 2024

Conversation

jwillmartin
Copy link
Contributor

PR Details

Description

These changes address a memory leak in the WebService/PSM functionality of the plugin. It also allows a user to switch between using FLIR(WebSocket) and PSM(WebService) without crashing or generating segmentation faults.

Related Issue

#497

Motivation and Context

Users of this plugin have noticed memory loss and error code generation while using this plugin. Users have also requested a fix for the #497 issue.

How Has This Been Tested?

Tested using a script to send 1000 PSM XMLs to the plugin. No messages were lost.
Tested using local FLIR camera to ensure functionality was maintained.

Switched between DataProviders (PSM/FLIR) in various combinations to ensure threads were closed or joined properly.

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.

@@ -633,7 +633,7 @@ int CommandPlugin::WSCallbackBASE64(
argsList[arg.first] = arg.second.data();
}
}
catch (exception argsEx)
catch (exception &argsEx)
Copy link
Contributor

Choose a reason for hiding this comment

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

catch(const std::exception &argEx)

Also where you just address a code smell or why are there changes to this file at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was just to address a code smell

	modified:   src/v2i-hub/PedestrianPlugin/src/PedestrianPlugin.cpp
	modified:   src/v2i-hub/PedestrianPlugin/src/include/PedestrianPlugin.hpp
Copy link

sonarcloud bot commented Jun 28, 2024

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarCloud

	modified:   examples/tmx-exampleapps/EmptyPlugin/src/EmptyPlugin.cpp
	modified:   ext/build.sh
	modified:   src/tmx/TmxApi/CMakeLists.txt
	modified:   src/tmx/TmxApi/tmx/messages/message_document.hpp
	deleted:    src/tmx/TmxApi/tmx/pugixml/pugiconfig.hpp
	deleted:    src/tmx/TmxApi/tmx/pugixml/pugixml.cpp
	deleted:    src/tmx/TmxApi/tmx/pugixml/pugixml.hpp
	deleted:    src/tmx/TmxApi/tmx/pugixml/readme.txt
@jwillmartin jwillmartin marked this pull request as draft August 2, 2024 14:53
	modified:   src/v2i-hub/PedestrianPlugin/src/PedestrianPlugin.cpp
	modified:   src/v2i-hub/PedestrianPlugin/src/include/PedestrianPlugin.hpp
@jwillmartin jwillmartin marked this pull request as ready for review August 12, 2024 12:52
ext/build.sh Outdated
@@ -72,3 +72,11 @@ make -j${numCPU}
make install
popd

# pugixml
pushd /tmp
git clone https://github.com/zeux/pugixml.git
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to target a specific release to avoid new releases breaking our build

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, changed to v1.14

Copy link
Contributor

@paulbourelly999 paulbourelly999 left a comment

Choose a reason for hiding this comment

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

I left several small comments. For ones labeled NIT please address or don't address these as you see fit. For all others please address them.

	modified:   src/v2i-hub/PedestrianPlugin/manifest.json
	modified:   src/v2i-hub/PedestrianPlugin/scripts/sendPSM.py
	modified:   src/v2i-hub/PedestrianPlugin/src/PedestrianPlugin.cpp
	modified:   src/v2i-hub/PedestrianPlugin/src/include/PedestrianPlugin.hpp
@jwillmartin jwillmartin requested review from paulbourelly999 and removed request for abey-yoseph August 16, 2024 14:26
@paulbourelly999 paulbourelly999 merged commit eb539af into develop Aug 16, 2024
1 of 2 checks passed
@paulbourelly999 paulbourelly999 deleted the fix-ped-plugin branch August 16, 2024 14:59
Copy link

sonarcloud bot commented Aug 16, 2024

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarCloud

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.

2 participants