Make priority attribute boolean and bugfixing traffic lights #385

Grufoony · 2025-12-22T16:43:05Z

No description provided.

codecov · 2025-12-22T16:57:35Z

Codecov Report

❌ Patch coverage is 97.91667% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.76%. Comparing base (953ae8d) to head (6a0955b).
⚠️ Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
src/dsf/mobility/RoadNetwork.cpp	95.15%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #385      +/-   ##
==========================================
+ Coverage   83.55%   84.76%   +1.21%     
==========================================
  Files          53       53              
  Lines        5466     5699     +233     
  Branches      634      640       +6     
==========================================
+ Hits         4567     4831     +264     
+ Misses        888      857      -31     
  Partials       11       11

Flag	Coverage Δ
unittests	`84.76% <97.91%> (+1.21%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

test/mobility/Test_graph.cpp

+      RoadNetwork graph{};
+      // Node 1 is the intersection
+      // Edge 1: 0 -> 1 (HIGHWAY)
+      Street s1(1, std::make_pair(0, 1), 100., 30., 1);


test/mobility/Test_graph.cpp

+      s1.setRoadType(RoadType::HIGHWAY);
+
+      // Edge 2: 2 -> 1 (HIGHWAY)
+      Street s2(2, std::make_pair(2, 1), 100., 30., 1);


test/mobility/Test_graph.cpp

+      s2.setRoadType(RoadType::HIGHWAY);
+
+      // Edge 3: 3 -> 1 (SECONDARY)
+      Street s3(3, std::make_pair(3, 1), 100., 30., 1);


Copilot

Pull request overview

This pull request refactors the road priority system from a numeric priority value (int m_priority) to a boolean flag (bool m_hasPriority) that indicates whether a road has priority or not. It also introduces a new method autoAssignRoadPriorities() that automatically assigns priorities based on road types at intersections.

Changes:

Changed priority from a numeric value to a boolean flag indicating presence/absence of priority
Updated autoMapStreetLanes() to use estimated flow (maxSpeed × nLanes) instead of numeric priorities
Added autoAssignRoadPriorities() method to automatically assign priorities based on road types

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/dsf/mobility/Road.hpp	Changed `m_priority` from `int` to `bool m_hasPriority`, updated `setPriority()` to take no parameters and `priority()` to `hasPriority()`
src/dsf/mobility/Road.cpp	Removed the constructor initialization of `m_priority` and the `setPriority(int)` method implementation
src/dsf/mobility/Street.cpp	Updated equality operator to compare `m_hasPriority` instead of `m_priority`
src/dsf/mobility/RoadNetwork.hpp	Added declaration for `autoAssignRoadPriorities()` method
src/dsf/mobility/RoadNetwork.cpp	Updated `autoMapStreetLanes()` to use estimated flow calculations; added `autoAssignRoadPriorities()` implementation
test/mobility/Test_street.cpp	Updated tests to use `hasPriority()` and parameter-less `setPriority()`
test/mobility/Test_graph.cpp	Added test case for `autoAssignRoadPriorities()`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/dsf/mobility/Road.hpp

Copilot · 2026-01-16T12:55:39Z

src/dsf/mobility/RoadNetwork.hpp

    /// @brief Automatically re-maps street lanes basing on network's topology
    /// @details For example, if one street has the right turn forbidden, then the right lane becomes a straight one
    void autoMapStreetLanes();



The new autoAssignRoadPriorities() method lacks documentation. Add a @brief comment explaining what this method does, such as '@brief Automatically assigns road priorities based on road types at intersections'.

Suggested change

/// @brief Automatically assigns road priorities based on road types at intersections.

src/dsf/mobility/RoadNetwork.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

github-advanced-security

Cppcheck (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 11 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-19T11:05:14Z

src/dsf/mobility/RoadNetwork.cpp

+            /*************************************************************
         * 2. Check for street names with same max speed
         * ***********************************************************/


The comment says "Check for street names with same max speed" but the logic is checking for streets with the highest speed, not streets with the same speed. The comment should be updated to reflect the actual behavior, e.g., "Check for streets with highest max speed".

Copilot · 2026-01-19T11:05:14Z

src/dsf/mobility/RoadNetwork.cpp

+            // Replace with a normal intersection
+            auto const& geometry{pNode->geometry()};
+            if (geometry.has_value()) {
+              pNode = std::make_unique<Intersection>(pNode->id(), *geometry);
+            } else {
+              pNode = std::make_unique<Intersection>(pNode->id());


Modifying pNode inside a parallel_for_each can cause a data race. At line 387 and 389, pNode is reassigned to a new unique_ptr, which modifies the map entry. If multiple threads are iterating over m_nodes and one thread modifies the map structure or a specific entry, this can lead to undefined behavior. Consider using a standard for_each loop for this function, or collect nodes to convert and process them sequentially after the parallel phase.

Suggested change

// Replace with a normal intersection

auto const& geometry{pNode->geometry()};

if (geometry.has_value()) {

pNode = std::make_unique<Intersection>(pNode->id(), *geometry);

} else {

pNode = std::make_unique<Intersection>(pNode->id());

// Not enough neighbours to configure a proper traffic light; skip further processing

auto const& geometry{pNode->geometry()};

if (geometry.has_value()) {

// In a parallel context, avoid modifying the underlying node container here.

} else {

// In a parallel context, avoid modifying the underlying node container here.

Copilot · 2026-01-19T11:05:14Z

src/dsf/mobility/RoadNetwork.cpp

+#include "RoadNetwork.hpp"
 #include "../geometry/Point.hpp"
 #include "../geometry/PolyLine.hpp"
-#include "RoadNetwork.hpp"

 #include <algorithm>
 #include <ranges>

 #include <rapidcsv.h>
 #include <simdjson.h>
+#include <tbb/parallel_for_each.h>


The include order has been changed: "RoadNetwork.hpp" is now first, followed by local includes, then standard library includes, then third-party includes. While this is a common pattern (own header first), make sure this aligns with the project's style guide. If the project previously grouped includes differently, this change could affect compilation order or cause issues with implicit dependencies.

Copilot · 2026-01-19T11:05:14Z

src/dsf/mobility/RoadNetwork.cpp

 #include <simdjson.h>
+#include <tbb/parallel_for_each.h>
+
+// Default traffic light cycle duration in seconds


The default traffic light cycle duration has been changed from 120 to 90 seconds. This is a significant behavioral change that affects traffic light initialization from CSV imports. Ensure this change is intentional and documented, as it will affect all traffic lights created during import operations. Consider whether this should be configurable or if existing configurations need to be migrated.

Suggested change

// Default traffic light cycle duration in seconds

// Default traffic light cycle duration in seconds for traffic lights created during

// CSV import operations. This value was intentionally changed from 120s to 90s to

// better match typical urban signal timing and simulation requirements. If existing

// scenarios relied on the previous 120s default, they should be updated explicitly

// in their configuration rather than depending on this hard-coded default.

Copilot · 2026-01-19T11:05:15Z

src/dsf/mobility/RoadNetwork.cpp

+            /*************************************************************
         * 2. Check for street names with same number of lanes
         * ***********************************************************/


The comment numbering is incorrect. Both the speed check (line 451) and the lane count check (line 461) are labeled as "2". The lane count check should be labeled as "3" since it's the third check, and the angle check should be labeled as "4".

Copilot · 2026-01-19T11:05:15Z

src/dsf/mobility/RoadNetwork.cpp

+            // Assign first and third keys of capacity map
+            auto it = capacities.begin();
+            auto const& firstKey = it->first;
+            ++it;
+            ++it;
+            auto const& thirdKey = it->first;
+            tl.addStreetPriority(firstKey);
+            tl.addStreetPriority(thirdKey);


This code assumes the capacities map has at least 3 elements (accessing first, incrementing twice, then accessing third). However, the check at line 380 only ensures there are at least 3 incoming neighbours, but if the angle-based or other priority selection methods succeeded but resulted in a count other than 2, this fallback code could fail with undefined behavior if capacities has fewer than 3 elements. Consider adding a check that capacities.size() >= 3 before this logic.

Suggested change

// Assign first and third keys of capacity map

auto it = capacities.begin();

auto const& firstKey = it->first;

++it;

++it;

auto const& thirdKey = it->first;

tl.addStreetPriority(firstKey);

tl.addStreetPriority(thirdKey);

// Assign priorities based on capacity map, guarding against small maps

if (capacities.size() >= 3) {

// Original behavior: use first and third keys

auto it = capacities.begin();

auto const& firstKey = it->first;

++it;

++it;

auto const& thirdKey = it->first;

tl.addStreetPriority(firstKey);

tl.addStreetPriority(thirdKey);

} else if (capacities.size() >= 2) {

// Fallback: use first two keys

auto it = capacities.begin();

auto const& firstKey = it->first;

++it;

auto const& secondKey = it->first;

tl.addStreetPriority(firstKey);

tl.addStreetPriority(secondKey);

} else if (!capacities.empty()) {

// Last resort: only one key available

tl.addStreetPriority(capacities.begin()->first);

}

Copilot · 2026-01-19T11:05:15Z

src/dsf/mobility/RoadNetwork.cpp

-        /*************************************************************
+          if (tl.streetPriorities().empty() && higherNLanes != lowerNLanes) {
+            /*************************************************************
         * 2. Check for street names with same number of lanes


The comment says "Check for street names with same number of lanes" but the logic is checking for streets with the highest number of lanes, not streets with the same count. The comment should be updated to reflect the actual behavior, e.g., "Check for streets with highest number of lanes".

Suggested change

* 2. Check for street names with same number of lanes

* 2. Check for streets with highest number of lanes

Copilot · 2026-01-19T11:05:16Z

src/dsf/mobility/RoadNetwork.hpp

    /// @brief Initialize the traffic lights with random parameters
-    /// @param minGreenTime The minimum green time for the traffic lights cycles (default is 30)
+    /// @param mainRoadPercentage The percentage of main roads for the traffic lights cycles (default is 0.6)
    /// @details Traffic Lights with no parameters set are initialized with random parameters.
    /// Street priorities are assigned considering the number of lanes and the speed limit.
    /// Traffic Lights with an input degree lower than 3 are converted to standard intersections.
-    void initTrafficLights(Delay const minGreenTime = 30);
+    void autoInitTrafficLights(double const mainRoadPercentage = 0.6);


The docstring says "Initialize the traffic lights with random parameters" but the function now uses a deterministic algorithm based on street characteristics (names, speeds, lanes, angles) with a fallback to capacity-based selection. The word "random" is misleading. Consider updating the description to something like "Automatically initialize traffic lights based on street characteristics".

Copilot · 2026-01-19T11:05:16Z

src/dsf/mobility/RoadNetwork.cpp

+      for (auto const& streetId : priorityRoads) {
+        auto const& pStreet{this->edge(streetId)};
+        pStreet->setPriority();
+        spdlog::debug("Setting priority to street {}", pStreet->id());
+      }


Calling setPriority() on edges from within a parallel_for_each can cause a data race if multiple threads attempt to modify the same edge. Although each node processes its own incoming edges, the same edge could theoretically be an incoming edge to multiple nodes being processed in parallel. Consider using a mutex to protect edge modifications or collect all edges to modify and process them sequentially after the parallel phase.

Copilot · 2026-01-19T11:05:16Z

src/dsf/mobility/RoadNetwork.cpp

+          if (tl.streetPriorities().empty() || tl.streetPriorities().size() != 2) {
+            spdlog::warn("Failed to auto-init Traffic Light {} - going random",
+                         pNode->id());
+            // Assign first and third keys of capacity map
+            auto it = capacities.begin();
+            auto const& firstKey = it->first;
+            ++it;
+            ++it;
+            auto const& thirdKey = it->first;
+            tl.addStreetPriority(firstKey);
+            tl.addStreetPriority(thirdKey);
          }


The condition now checks if the size is not exactly 2, which is a new requirement. However, if the number of street priorities is already greater than 2 (e.g., 3 or 4), this will trigger the "going random" logic and add two more priorities, leading to duplicate priorities or an incorrect total count. Consider only adding the fallback priorities if the set is empty, or first clearing existing priorities before adding new ones.

Grufoony force-pushed the priority branch from 6182b83 to 14d7543 Compare January 16, 2026 12:11

Grufoony added 3 commits January 16, 2026 13:16

Init refactoring

f9869d0

Better handling of nlanes

8a18a08

Fix

4963bd2

Grufoony force-pushed the priority branch from 14d7543 to 4963bd2 Compare January 16, 2026 12:16

Grufoony added 3 commits January 16, 2026 13:16

Init auto priority assignment

cc10361

Merge branch 'autopriority' into priority

e8b1728

Fix auto algorithm

2dfc206

github-advanced-security bot found potential problems Jan 16, 2026

View reviewed changes

Grufoony requested a review from Copilot January 16, 2026 12:53

Copilot started reviewing on behalf of Grufoony January 16, 2026 12:53 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

Grufoony and others added 5 commits January 16, 2026 14:38

Update src/dsf/mobility/Road.hpp

b55c509

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/dsf/mobility/RoadNetwork.cpp

2ac0b7a

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Refactor autoinittrafficlights function

e2c784e

Formatting

91bcf15

Add tests

d07ff2c

Grufoony requested a review from Copilot January 19, 2026 11:00

Copilot started reviewing on behalf of Grufoony January 19, 2026 11:00 View session

github-advanced-security bot found potential problems Jan 19, 2026

View reviewed changes

Copilot AI reviewed Jan 19, 2026

View reviewed changes

Grufoony and others added 2 commits January 19, 2026 16:17

Bugfix

1bcbb3e

Bump version

6a0955b

Grufoony changed the title ~~Priority~~ Make priority attribute boolean and bugfixing traffic lights Jan 20, 2026

Grufoony merged commit eb83c88 into main Jan 20, 2026
47 checks passed

Grufoony deleted the priority branch January 20, 2026 11:35



	/// @brief Automatically assigns road priorities based on road types at intersections.

-// Default traffic light cycle duration in seconds
+// Default traffic light cycle duration in seconds for traffic lights created during
+// CSV import operations. This value was intentionally changed from 120s to 90s to
+// better match typical urban signal timing and simulation requirements. If existing
+// scenarios relied on the previous 120s default, they should be updated explicitly
+// in their configuration rather than depending on this hard-coded default.

	* 2. Check for street names with same number of lanes
	* 2. Check for streets with highest number of lanes

Make priority attribute boolean and bugfixing traffic lights #385

Make priority attribute boolean and bugfixing traffic lights #385

Conversation

Grufoony commented Dec 22, 2025

Uh oh!

codecov bot commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Check notice

Check notice

Check notice

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-advanced-security bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Dec 22, 2025 •

edited

Loading