AER-4271 Added support for using max speed with NON_URBAN ROADS#379
AER-4271 Added support for using max speed with NON_URBAN ROADS#379Hilbrand wants to merge 3 commits intoaerius:mainfrom
Conversation
With new road emissions for NON_URBAN_ROAD speed will be relevant. Therefore speed will be read. Because with new data speed determines the emission and not only the road type code, the road type code has been generalized to NON_URBAN_ROAD in the GML. The road type code NON_URBAN_ROAD_NATIONAL and NON_URBAN_ROAD_GENERAL could be used on both SRM1 and SRM2 soruces. But these with the new data they become explicitly for use with a specific SRM version. NON_URBAN_ROAD_NATIONAL is for SRM2 roads, and NON_URBAN_ROAD_GENERAL for SRM1 roads. This was somewhat implicit, as in Calculator UI only NON_URBAN_ROAD_NATIONAL was used, and only SRM2 can be modified in Calculator. But it would be possible to manually use NON_URBAN_ROAD_GENERAL in an SRM2 source. Both NON_URBAN_ROAD_NATIONAL and NON_URBAN_ROAD_GENERAL are still supported to be used in GML, also for backward compatibility.
BertScholten
left a comment
BertScholten
left a comment
There was a problem hiding this comment.
Not a fan of the temporary bit, but otherwise LGTM
| standardVehicle.getValuesPerVehicleTypes().put(sv.getVehicleType().getStandardVehicleCode(), valuesPerVehicleType); | ||
| } | ||
|
|
||
|
|
| emissionSource.setTrafficDirection(source.getTrafficDirection()); | ||
| emissionSource.setRoadManager(source.getRoadManager()); | ||
| emissionSource.setRoadAreaCode("NL"); | ||
| // Ensure road type get set before specific, as it's overwritten by SRM1 |
There was a problem hiding this comment.
Might make more sense to move this comment as well, or remove it completely.
There was a problem hiding this comment.
A good point. This can actually be simplified by just moving the code that is set in the specific in the convertRoadTypeCode
| final boolean maxSpeedNotSet = vehicles.stream().filter(StandardVehicles.class::isInstance).map(StandardVehicles.class::cast) | ||
| .anyMatch(e -> Optional.ofNullable(e.getMaximumSpeed()).orElse(0) == 0); | ||
|
|
||
| return !maxSpeedNotSet && roadTypeCode.startsWith(RoadType.NON_URBAN_ROAD.name()) ? RoadType.NON_URBAN_ROAD.name() : roadTypeCode; |
There was a problem hiding this comment.
This means that if we have a release of calculator before next actualisation release (when all these sources should get a max speed), all exported GMLs for non urban roads will get NON_URBAN_ROAD as road type? And after the next actualisation release they once again will use NON_URBAN_ROAD_GENERAL or NON_URBAN_ROAD_NATIONAL?
Hope no other release in that case.
There was a problem hiding this comment.
No (at least that is not the idea of this implementation). At any moment (and this can be before the actualisation release when the current data would be used) if no speed is set or if speed is 0 it will keep the NON_URBAN_ROAD_GENERAL or NON_URBAN_ROAD_NATIONAL (those are used internally to specify road type). At any point when it would have a speed > 0 it will set NON_URBAN_ROAD. In the future unless we get non urban road with no speed again the NON_URBAN_ROAD_GENERAL or NON_URBAN_ROAD_NATIONAL should not be set anymore.
The idea is in GML it can have:
- no speed or 0 ->
NON_URBAN_ROAD_GENERALorNON_URBAN_ROAD_NATIONAL - with speed > 0 ->
NON_URBAN_ROAD
Does this seem to match with what you see in the code?
I'll rewrite the boolean check to be inverted: instead of any use all. That will make it a bit more readable.
There was a problem hiding this comment.
Ah OK, the doube negation did get me there. But if that's the case, then the codes in the database no longer match those in GML, that seems a bit of a drawback? Or did that get changed in later data supply? (ones I got so far kept the same road types).
And when importing its setting a max speed anyway, so there is already going something awry anyway if you export->import->export? If you create a new source in calculator now, it'll get NON_URBAN_ROAD_NATIONAL with 0 max speed in the exported GML. If you then import it gets max speed 80 due to GML2Road, and when you export that again you get NON_URBAN_ROAD with 80 max speed?
There was a problem hiding this comment.
I added the NON_URBAN_ROAD to make explicit separation between old and new. old is without speed and new with speed. That way when NON_URBAN_ROAD is present it indicates a speed is to be set. But on second thought this might not add much. Therefore I removed NON_URBAN_ROAD and kept the original only. This still means when reading old files with speed not set or 0 it will get a speed value (60 or 80) because when new emission factors are present it must have a speed. To be already compatible with that situation it will already set the speed when importing. For current behaviour this has no impact, other than when exporting the new speed value is set. Newly created sources in the UI do get 0 currently and 0 will be writtten. But that is no problem. Because when imported again (in the future) the speed will be set.
- Simplified GML2SRMRoad v4 implementation by merging specific variables with convertRoadType because specificVariables was only used for road type, and since both methods have a SRM specific implementation the SRM1 case can be moved to the convertRoadTypeCode. - In Road2GML non urban road type conversion inversed check on if 0 speed present from any to all such that no negative check has to be done
…AL/GENERAL) only. When importing always set a road speed for NON_URBAN_ROAD when road is 0 (or not set) to representative speeds.
3 participants
With new road emissions for NON_URBAN_ROAD speed will be relevant. Therefore speed will be read. Because with new data speed determines the emission and not only the road type code, the road type code has been generalized to NON_URBAN_ROAD in the GML. The road type code NON_URBAN_ROAD_NATIONAL and NON_URBAN_ROAD_GENERAL could be used on both SRM1 and SRM2 soruces. But these with the new data they become explicitly for use with a specific SRM version. NON_URBAN_ROAD_NATIONAL is for SRM2 roads, and NON_URBAN_ROAD_GENERAL for SRM1 roads. This was somewhat implicit, as in Calculator UI only NON_URBAN_ROAD_NATIONAL was used, and only SRM2 can be modified in Calculator. But it would be possible to manually use NON_URBAN_ROAD_GENERAL in an SRM2 source. Both NON_URBAN_ROAD_NATIONAL and NON_URBAN_ROAD_GENERAL are still supported to be used in GML, also for backward compatibility.