Skip to content

Conversation

@nikolavasiljevski
Copy link
Contributor

Added missing Main and Supplementary signs from StVO.
Used https://www.adac.de/_mmm/pdf/fi_verkehrszeichen_engl_infobr_0915_30482.pdf as base for adding new signs.
Also included addition sub-types found at : https://de.wikipedia.org/wiki/Bildtafel_der_Verkehrszeichen_in_der_Bundesrepublik_Deutschland_seit_2017

@ghost ghost changed the title Extended osi_trafficsign.proto with missing signs Extended osi_trafficsign.proto with missing signs fixes #261 Aug 3, 2018
@ghost ghost added the FeatureRequest Proposals which enhance the interface or add additional features. label Aug 6, 2018
@ghost ghost added this to the v3.1.0 milestone Aug 6, 2018
@ghost ghost changed the title Extended osi_trafficsign.proto with missing signs fixes #261 Extended osi_trafficsign.proto with missing signs fixes #261 fixes #241 Aug 6, 2018
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Hello Nikola, I finally finished my review.
It has taken some time to check all the numbers, the statements, the translation and the consistency. I hope this is helpfull. Regards, Ludwig

@@ -960,216 +960,2890 @@ message TrafficSign
// See e.g.:
//
TYPE_TRAFFIC_LIGHT_GREEN_ARROW = 92;

Copy link

Choose a reason for hiding this comment

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

Please remove every "ahead" in comments and enumerations. It's inconsistently used and doesn't possess additional value.
Please remove every "warning" in comments and enumerations. It's inconsistently used and doesn't possess additional value.
Please replace "banned" with "prohibited" in comments and enumerations.
Please replace "allowed" with "only" in enumerations (even if otherwise stated by me).

//
// See e.g.:
//
TYPE_LOOSE_CHIPPINGS = 97;
Copy link

Choose a reason for hiding this comment

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

loose chipping -> loose gravel

//
// See e.g.:
//
TYPE_SWING_BRIDGE_AHEAD = 101;
Copy link

Choose a reason for hiding this comment

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

swing bridge -> lifting bridge

//
// See e.g.: https://www.dvr.de/bilder/stvo/gt/142-10.png
//
TYPE_WILD_ANIMALS_CROSSING_WARNING_RIGHT = 113;
Copy link

Choose a reason for hiding this comment

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

wild animal crossing -> deer crossing

//
// See e.g.:
//
TYPE_WILD_ANIMALS_CROSSING_WARNING_LEFT = 114;
Copy link

Choose a reason for hiding this comment

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

wild animal crossing -> deer crossing

//
// See e.g.:
//
TYPE_FL
Copy link

Choose a reason for hiding this comment

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

within marked parking areas only -> marked parking areas only

//
// See e.g.:
//
TYPE_FL
Copy link

Choose a reason for hiding this comment

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

no parking on shoulder -> no waiting side stripes

//
// See e.g.:
//
TYPE_FL
Copy link

Choose a reason for hiding this comment

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

also buses and cars with trailers ->cars trailer buses additionally

//
// See e.g.:
//
TYPE_FL
Copy link

Choose a reason for hiding this comment

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

endangered area rabies -> rabies

//
// See e.g.:
//
TYPE_FL
Copy link

Choose a reason for hiding this comment

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

endangered area wild animals rabies -> wild animals rabies

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Hello Nikola, I finally finished my review.
It has taken some time to check all the numbers, the statements, the translation and the consistency. I hope this is helpfull. Regards, Ludwig

Copy link
Contributor Author

@nikolavasiljevski nikolavasiljevski left a comment

Choose a reason for hiding this comment

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

Hello Ludwig,

Thanks for the review, it was helpful.
I made all the changes suggested, will commit them after this review.
Also I added 3 comments, for further clarification.

Best Regards,
Nikola

Nikola Vasiljevski added 2 commits August 21, 2018 18:04
 - Fixed enum names
 - Types distinguished
 - Translation fixed
 - Extra signs removed
 - Enum values aligned
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Why did you (re)append 'arrow'?

@nikolavasiljevski
Copy link
Contributor Author

That was a mistake, sorry. I removed it.
I had some issues with duplicated enum names and when doxygen test failed, I automatically assumed that it was the case and added ARROW.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Hi Nikola,
I reviewed till line 3226. At that point, I realized the issues I pointed out in my last review were not adressed. Please check my first review,
Regards Ludwig

//
// See e.g.:
//
TYPE_RISK_OF_SHOW_OR_ICE = 94;
Copy link

Choose a reason for hiding this comment

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

risk of show or ice -> snow or ice

//
// See e.g.: https://www.dvr.de/bilder/stvo/gt/156-10.png
//
TYPE_LEVER_CROSSING_COUNTDOWN_RIGHT = 118;
Copy link

Choose a reason for hiding this comment

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

level

//
// See e.g.:
//
TYPE_LEVER_CROSSING_COUNTDOWN_LEFT = 119;
Copy link

Choose a reason for hiding this comment

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

level

//
// See e.g.:
//
TYPE_MOPEDS_PROHIBITED = 155;
Copy link

Choose a reason for hiding this comment

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

mopeds prohibited -> small mopeds prohibited

//
// See e.g.:
//
TYPE_PLACE_NAME_SIGN = 216;
Copy link

Choose a reason for hiding this comment

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

place name sign -> place name

TYPE_ZONE = 20;

// Stop 4 way.
//
Copy link

Choose a reason for hiding this comment

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

TYPE_KNOLL -> TYPE_HILLTOP

TYPE_ZONE = 20;

// Stop 4 way.
//
Copy link

Choose a reason for hiding this comment

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

TYPE_WINTER_SPORTS_ALONG_THE_ROAD -> TYPE_WINTER_SPORTS

TYPE_ZONE = 20;

// Stop 4 way.
//
Copy link

Choose a reason for hiding this comment

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

TYPE_LARGE_WAGONS_PARK_OVER_2_WEEKS -> TYPE_LONG-TERM_PARKING_TRAILERS

Copy link
Contributor Author

@nikolavasiljevski nikolavasiljevski Aug 28, 2018

Choose a reason for hiding this comment

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

'-' is invalid character. I will use _ instead.

TYPE_ZONE = 20;

// Stop 4 way.
//
Copy link

Choose a reason for hiding this comment

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

TYPE_CARAVAN_PARK_OVER_2_WEEKS -> TYPE_LONG-TERM_PARKING_CARAVANS

TYPE_ZONE = 20;

// Stop 4 way.
//
Copy link

Choose a reason for hiding this comment

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

I think I already reviewed the following lines. Please check my first review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was changing Allowed to Only and I missed most of the comments for supplementary signs.
I went over them again and made changes.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Hi Nikola, that's it - I finished my second review.

Regards,
Ludwig


// Restriction of the validity of the traffic sign depending on
// the vehicle's weight (more than \c TrafficSignValue::value
// e.g. in kilogram - \c TrafficSignValue::valu
Copy link

Choose a reason for hiding this comment

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

TYPE_DISABLED_PERSONS_PARKING_PERMIT_ALLOWED -> TYPE_DISABLED_PERSONS_PARKING_PERMIT_ONLY


// Restriction of the validity of the traffic sign depending on
// the vehicle's weight (more than \c TrafficSignValue::value
// e.g. in kilogram - \c TrafficSignValue::valu
Copy link

Choose a reason for hiding this comment

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

TYPE_WINTER_SPORTS_ALLOWED -> TYPE_WINTER_SPORTS_ONLY


// Restriction of the validity of the traffic sign depending on
// the vehicle's weight (more than \c TrafficSignValue::value
// e.g. in kilogram - \c TrafficSignValue::valu
Copy link

Choose a reason for hiding this comment

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

TYPE_RESIDENTS_ALLOWED -> TYPE_RESIDENTS_ONLY


// Restriction of the validity of the traffic sign depending on
// the vehicle's weight (more than \c TrafficSignValue::value
// e.g. in kilogram - \c TrafficSignValue::valu
Copy link

Choose a reason for hiding this comment

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

TYPE_RESIDENTS_PARKING_ALLOWED -> TYPE_RESIDENTS_PARKING_ONLY


// Restriction of the validity of the traffic sign depending on
// the vehicle's weight (more than \c TrafficSignValue::value
// e.g. in kilogram - \c TrafficSignValue::valu
Copy link

Choose a reason for hiding this comment

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

TYPE_RESIDENTS_PARKING_PERMIT_ALLOWED -> TYPE_RESIDENTS_PARKING_PERMIT_ONLY


// Restriction of the validity of the traffic sign depending on
// the vehicle's weight (more than \c TrafficSignValue::value
// e.g. in kilogram - \c TrafficSignValue::valu
Copy link

Choose a reason for hiding this comment

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

TYPE_TAXI -> TYPE_TAXI_ONLY


// Restriction of the validity of the traffic sign depending on
// the vehicle's weight (more than \c TrafficSignValue::value
// e.g. in kilogram - \c TrafficSignValue::valu
Copy link

Choose a reason for hiding this comment

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

TYPE_TAXI_QUANTITY -> TYPE_TAXI_QUANTITY_ONLY


// Restriction of the validity of the traffic sign depending on
// the vehicle's weight (more than \c TrafficSignValue::value
// e.g. in kilogram - \c TrafficSignValue::valu
Copy link

Choose a reason for hiding this comment

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

TYPE_ELECTRIC_VEHICLES_CHARGING -> TYPE_ELECTRIC_VEHICLES_CHARGING_ONLY


// Restriction of the validity of the traffic sign depending on
// the vehicle's weight (more than \c TrafficSignValue::value
// e.g. in kilogram - \c TrafficSignValue::valu
Copy link

Choose a reason for hiding this comment

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

TYPE_ELECTRIC_VEHICLES -> TYPE_ELECTRIC_VEHICLES_ONLY


// Restriction of the validity of the traffic sign depending on
// the vehicle's weight (more than \c TrafficSignValue::value
// e.g. in kilogram - \c TrafficSignValue::valu
Copy link

Choose a reason for hiding this comment

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

TYPE_PARKING_TICKET -> TYPE_PARKING_TICKET_ONLY

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Hi Nikola,
Looks good, thanks for your patience!

Regards,
Ludwig

@carsten-kuebler
Copy link
Contributor

@LudwigFriedmannBMW @nikolavasiljevski
Hello Ludwig,

I checked the changes.

I would propose that:

  1. Traffic signs e.g. 625 should use direction_scope and not two different enumerators TYPE_BEND_INDICATOR_LEFT = 308; TYPE_BEND_INDICATOR_RIGHT = 309;

  2. I think Nikola changed the meaning of the supplementary signs:
    For example
    // Only motorcycles allowed.
    //
    TYPE_MOTORCYCLE_ALLOWED = 23;

            // Motorcycles only.
            // (StVO 1022-12).
            //
            // See e.g.:
            //
            TYPE_MOTORCYCLES_ONLY = 23;
    

The meaning of "Motorrad frei" is not "motorcycles only", it is "except motorcycles" and therefore "allowed for motorcycles".

  1. If we change the names of enumerators, this PR could only be merged in V4.0.0

@LudwigFriedmannBMW shall we (TWT) modify the proposed PR?

@ghost
Copy link

ghost commented Sep 13, 2018

Hi @carsten-kuebler,
translating the expression "frei" to "only" instead of "excluded" and merging these with "only" signs was my idea & fault.

Nevertheless, no existing enumerators should have been changed or overridden.

So this PR is an extension and may be integrated in OSI 3.1.0.

Please change the "frei" signs and the subtypes including a direction appropriately and merge the PR.

Regards,
Ludwig

@carsten-kuebler
Copy link
Contributor

@LudwigFriedmannBMW We will also reorder the enumerators so that the enumerators would be ordered by the stvo number. We will give feedback if we have additional comments.

@carsten-kuebler
Copy link
Contributor

@nikolavasiljevski and @LudwigFriedmannBMW

we have taken a look at the proposal by Nikola and have performed a number of corrections.

• Adjusted the ordering of the symbols: for the editor's convenience, we have resorted the symbols according to their StVO correspondence. Some entries cannot directly be sorted according to StVO. We have left those symbols at the end of the list and ordered with respect to their entry number (only for supplementary traffic signs). For main signs there are traffic signs without StVO to complete the list (e.g. U-turn left StVO 272, U-turn right not part of StVO).
• Eliminated some entries that were not part of the basic StVO and not relevant to the driving function (“Seuchengefahrhinweise”). See open question below.
• Edited some of the enumerator descriptions for additional understandability.
• Set plural names for actors: children, cars, taxis and so on.
• Merged entries that already existed with those that were newly introduced. Removed doubled definitions and used more detailed descriptions (adjusted entry numbers to previously defined values).
• Introduced a new comment at the end of the enumeration TYPE within Classification messages for both the MainSign and the SupplementarySign messages in which a counter called "last_counter" is introduced. This shows the highest value of the enumerators included and is thought to help in the processes of addition of new enumerators.
• We have merged the existing “TYPE_ZONE” with the newly proposed “TYPE_TRAFFIC_BAN_EXCEPTION_RED_YELLOW_GREEN “
• StVO 1010-12 “Long term parking trailers” and StVO 1010-13 “Long term parking caravans”. Seem to have a complete meaning by themselves. Additionally, they implicitly free from a parking constraint. Are they really supplementary signs/what is the main sign to these supplementary signs? Do we need to include the implicit semantic to the enumerator names (e.g. long term parking)?
• For the family StVO 315, replaced the word “FRONT” by “PERPENDICULAR” in the enumerator names.
• In some cases, such as the signs denoting „Electric Vehicles” (StVO 1010-66, 1050-33), there exist both a sign consisting of pure text and one of a pure image whereby only one semantic meaning is represented. We have merged these enumerators with one another in order to include the class representant only (similar to OSI 3.0).

@carsten-kuebler
Copy link
Contributor

@LudwigFriedmannBMW

we are preparing a new version of the osi_trafficsign.proto file in which we would like to introduce the changes proposed next:
• Navigation guides: for StVO 157 to 162, the current implementation includes different entries according to the placement position of the rod, and to the number of stripes it has. We believe the required information is provided by a single type together with a single TrafficSignValue object (number of stripes, without unit, or the indicated distance with its units) as StVO 450. We would propose to reduce the number of enumerators. In contrast to StVO 450 the distance is optional.
• Time-restriction signs: Several signs of the family StVO 1042 and some of the family StVO 1040 can be modelled with the use of a few basic building blocks such as StVO 1040-30, 1042-30 and 1042-30, together with other already available objects (e.g. TrafficSignValues). Hence, we think it is not necessary to introduce one enumerator per member of the aforementioned family and would propose an adequate use of those modelling elements to achieve a more compact list of enumerators.
• Parking symbols: Symbols of the family StVO 315 can be conveniently grouped instead of adding one enumerator per family member.
• Symmetric symbols: Several symbols are symmetric and thus have an identical semantic, but are associated to different StVO-numbers, cf. StVO 117-10 and 117-20, StVO 134-10 and 134-20, StVO 133-10 and 133-20 and many others. We believe it not necessary to have all those symbols repeated several times in our proto-file and thus we propose the use of an optional direction indicator only when such a differentiation is strictly required.
• Symbols and strings: We would like to propose two improvements

  1. we could model signs with an arbitrary text by introducing an adequate object to our TrafficSignValue message. This could importantly expand the standard’s flexibility and we would reduce the number of required enumerators.
  2. we would introduce a second type, similar as for the message RoadMarking, which indicates if the traffic sign consists only of text, or only of a symbol. That would allow us to capture additional information about a given sign.

We have also found a mistake from our side. Namely, the one-way signs of family StVO 220 are better modelled by means of the DirectionScope element. This should be changed for the next version.

We also have an open question for further discussion
• Should further warning signs such as “Lights on” (see WH 55) be also included? (For reference see https://www.strassenausstatter.de/kategorie/verkehrszeichen/warnschilderundhinweisschilder/page/4/ )

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

As discussed with @carsten-kuebler TWT will reduce and simplify the set of signs

Add BASt images.
Homogenized types.
@carsten-kuebler
Copy link
Contributor

@CarloVanDriestenBMW please check the actual Version with all the Images from BASt. There are still a lot of main signs and supplementary signs. We should reduce the number of types, shouldn't we?

@carsten-kuebler
Copy link
Contributor

Done:
• Navigation guides: for StVO 157 to 162, the current implementation includes different entries according to the placement position of the rod, and to the number of stripes it has. We believe the required information is provided by a single type together with a single TrafficSignValue object (number of stripes, without unit, or the indicated distance with its units) as StVO 450. We would propose to reduce the number of enumerators. In contrast to StVO 450 the distance is optional.
• Symmetric symbols: Several symbols are symmetric and thus have an identical semantic, but are associated to different StVO-numbers, cf. StVO 117-10 and 117-20, StVO 134-10 and 134-20, StVO 133-10 and 133-20 and many others. We believe it not necessary to have all those symbols repeated several times in our proto-file and thus we propose the use of an optional direction indicator only when such a differentiation is strictly required.

Additional:
• Add BASt images to OSI.

Open:
• Time-restriction signs: Several signs of the family StVO 1042 and some of the family StVO 1040 can be modelled with the use of a few basic building blocks such as StVO 1040-30, 1042-30 and 1042-30, together with other already available objects (e.g. TrafficSignValues). Hence, we think it is not necessary to introduce one enumerator per member of the aforementioned family and would propose an adequate use of those modelling elements to achieve a more compact list of enumerators.
• Parking symbols: Symbols of the family StVO 315 can be conveniently grouped instead of adding one enumerator per family member.
• Symbols and strings: We would like to propose two improvements
we could model signs with an arbitrary text by introducing an adequate object to our TrafficSignValue message. This could importantly expand the standard’s flexibility and we would reduce the number of required enumerators.
We would introduce a second type, similar as for the message RoadMarking, which indicates if the traffic sign consists only of text, or only of a symbol. That would allow us to capture additional information about a given sign.
We have also found a mistake from our side. Namely, the one-way signs of family StVO 220 are better modelled by means of the DirectionScope element. This should be changed for the next version.

We also have an open question for further discussion
• Should further warning signs such as “Lights on” (see WH 55) be also included? (For reference see https://www.strassenausstatter.de/kategorie/verkehrszeichen/warnschilderundhinweisschilder/page/4/ )

images in their original size can be downloaded directly
from (last query: 29 October 2018)

https://www.bast.de/BASt_2017/DE/Verkehrstechnik/Fachthemen/v1-verkehrszeichen/vz-download.html
Copy link

Choose a reason for hiding this comment

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

markdown

@ghost
Copy link

ghost commented Nov 16, 2018

We decided that the reasoning for certain decission e.g. introducint the vertical_mirror should be included in the description as readers normally do not follow the discussions within the PRs od issues.

@ghost
Copy link

ghost commented Nov 16, 2018

Optional: Add some more tests in python script and add clang

Update.
Add Arrow and Actor to supplementary sign. Not added to main sign because of leading to too many depreciated types. Will be proposed to change in OSI 4.0.
carsten-kuebler and others added 2 commits December 9, 2018 18:14
Small changes in documentation.
Change doxygen config file to use brief section.
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

build fail

@carsten-kuebler
Copy link
Contributor

@CarloVanDriestenBMW Build Failed

$ python test_cases_doc.py
doc/html/structosi3_1_1EnvironmentalConditions.html in line 232: not permitted hash found. Search for: ' #AMBIENT_ILLUMINATION_LEVEL1
doc/html/structosi3_1_1EnvironmentalConditions.html in line 265: not permitted hash found. Search for: ' #AMBIENT_ILLUMINATION_LEVEL9

please update proto2cpp PR

@carsten-kuebler
Copy link
Contributor

@CarloVanDriestenBMW I restart CI

Update documentation. Add "." at the end of a sentence.
@carsten-kuebler
Copy link
Contributor

@CarloVanDriestenBMW Test is now ok. I add some hundred "." at the end of some sentences...

Re-Format of clang
ghost
ghost previously requested changes Dec 12, 2018
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Please check WARNING and DANGER prefixes for consistency.
As they're yet not present in all warning signs, and as all warning sign enums make perfect sense without theses prefixes, in my opinion, they can be removed.

@@ -418,7 +418,7 @@ message TrafficSign
//
TYPE_SNOW_OR_ICE = 94;
Copy link

Choose a reason for hiding this comment

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

DANGER is missing here.
I'd propose to handle this consistently and thuss remove DANGER from all types

@@ -458,7 +458,7 @@ message TrafficSign
//
TYPE_DANGER_MOVABLE_BRIDGE = 101;
Copy link

Choose a reason for hiding this comment

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

Shouldn't this be LIFTING_BRIDGE?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is ist always lifting? Some rotate etc.

Copy link

Choose a reason for hiding this comment

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

Well, the sign depicts a lifting bridge and it's named lifting bridge.

Copy link

Choose a reason for hiding this comment

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

I am actually a litle bit on the side of "generic expression including all kinds of moving bridges". I would assume that the "moving" part is what is relevant for the function. In the end all different signs for "moving bridges" can be expressed with one type?

@@ -468,7 +468,7 @@ message TrafficSign
//
TYPE_RIGHT_BEFORE_LEFT_NEXT_INTERSECTION = 3;
Copy link

Choose a reason for hiding this comment

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

In the following defines, DANGER is missing again.
I'd propose to handle this consistently and thuss remove DANGER from all types.

@@ -579,7 +579,7 @@ message TrafficSign
//
TYPE_WARNING_ROAD_NARROWING = 10;
Copy link

Choose a reason for hiding this comment

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

Now WARNING steps in for DANGER.
Where's the difference? Is there any criterion to distinguish warning and danger?
Again, I would propose to remove it.

@@ -902,7 +902,7 @@ message TrafficSign
//
TYPE_PRESCRIBED_STRAIGHT_AHEAD = 22;
Copy link

Choose a reason for hiding this comment

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

TYPE_PRESCRIBED_STRAIGHT would be sufficient.

@@ -933,11 +933,11 @@ message TrafficSign
//
// As symbolic road marking
// \c RoadMarking::Classification::TYPE_SYMBOLIC_TRAFFIC_SIGN
// (StVO 297)
// (StVO 297).
//
TYPE_PRESCRIBED_RIGHT_TURN_AND_STRAIGHT_AHEAD = 26;
Copy link

Choose a reason for hiding this comment

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

Again, please remove AHEAD

@@ -948,11 +948,11 @@ message TrafficSign
//
// As symbolic road marking
// \c RoadMarking::Classification::TYPE_SYMBOLIC_TRAFFIC_SIGN
// (StVO 297)
// (StVO 297).
//
TYPE_PRESCRIBED_LEFT_TURN_AND_STRAIGHT_AHEAD = 25;
Copy link

Choose a reason for hiding this comment

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

Again, please remove AHEAD

@@ -971,11 +971,11 @@ message TrafficSign
//
// As symbolic road marking
// \c RoadMarking::Classification::TYPE_SYMBOLIC_TRAFFIC_SIGN
// (StVO 297)
// (StVO 297).
//
TYPE_PRESCRIBED_LEFT_TURN_RIGHT_TURN_AND_STRAIGHT_AHEAD = 28;
Copy link

Choose a reason for hiding this comment

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

Again, please remove AHEAD

@@ -2124,7 +2119,7 @@ message TrafficSign
//
TYPE_TRAFFIC_CALMED_DISTRICT_BEGIN = 77;
Copy link

Choose a reason for hiding this comment

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

Is there a better translation?

@@ -2135,7 +2130,7 @@ message TrafficSign
// Begin: \c #TYPE_TRAFFIC_CALMED_DISTRICT_BEGIN
TYPE_TRAFFIC_CALMED_DISTRICT_END = 78;
Copy link

Choose a reason for hiding this comment

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

Is there a better translation?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

If it is the official name according to Wikipedai -> Go for it :)

@ghost
Copy link

ghost commented Dec 12, 2018

I guess the inconsistency is "TYPE_DANGER_FALLING_ROCKS" , "TYPE_SNOW_OR_ICE", "TYPE_ROAD_WORK", "TYPE_WARNING_ROAD_NARROWING_LEFT " maybe it can just be altered in the 4.0.0

I added a comment in the corresponding issue

@ghost
Copy link

ghost commented Dec 12, 2018

Please ignore my comments if they affect types that were not introduced within this PR.

Update documentation. Unify zebra crossing
@carsten-kuebler
Copy link
Contributor

@LudwigFriedmannBMW: type names has changed for OSI 3.1. Names are consistent as ypu proposed.
@CarloVanDriestenBMW: Minor release: binary compatibility requried. Src. code compatibility is not granted.

Use styleguide for comments
@ghost ghost changed the title Extended osi_trafficsign.proto with missing signs fixes #261 fixes #241 Extended osi_trafficsign.proto with missing signs (fixes #261, fixes #241, fixes #281) Dec 14, 2018
@ghost ghost dismissed their stale review December 14, 2018 14:29

All included

@ghost ghost changed the title Extended osi_trafficsign.proto with missing signs (fixes #261, fixes #241, fixes #281) Extended osi_trafficsign.proto with missing signs (fixes #261, fixes #241, fixes #281, fixes #219) Dec 14, 2018
@ghost ghost merged commit ce92c01 into OpenSimulationInterface:master Dec 14, 2018
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FeatureRequest Proposals which enhance the interface or add additional features.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants