CS-57: Fix threading for TSC and fix CI for bionic nodejs version incompatibility #421
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Details
Description
This PR addresses to regressions in the TSC Service.
The first of which is related to launching the threads necessary for consuming a desired phase plan and making the appropriate NTCIP 1202 calls to a traffic signal controller (TSC). We use
std::thread
s for this and call thejoin()
method on each instead ofdetach()
to launch all our threads ( See sonar scanner rule and CPP Core Guidelines to see why we use join instead of detach). The std::thread::join() call blocks until the referenced thread completes execution. Due to this , alljoin()
calls need to be at the end of their scope. Inserting ajoin()
call before some other logic will prevent that logic from executing until the joined thread completes execution, which for many of these threads will only happen when the service shuts down.As part of some MMITSS development this join was moved into the incorrect location causing the
control_tsc_phases
thread never to be created. Moving all thejoin()
calls to the end of the start method allows all threads to run and no logic to be blocked by thread execution completion.The second more benign regression was that we changed the SNMP COMMAND LOGGER to take it's name from a constant. In doing so, the name changes and calls to retrieve this logger did not use the correct name. This was also fixed in this PR
The final change is exclusively to fix issues with the CI process related to bionic (ubuntu distribution) incompatibility with many github actions due to its incompatibility with newer versions of nodejs. This environment variable allow github runners to use deprecated versions of nodesJS. Eventually we plan to remove bionic from CARMA Streets.
Related GitHub Issue
Related Jira Key
CS-57
Motivation and Context
Hot fix for carma-system-4.5.0 for TSC Service. Fixing functionality to control phases given a desired phase plan.
How Has This Been Tested?
Tested in CDASim deployment
Types of changes
Checklist: