Skip to content

Conversation

@carsten-kuebler
Copy link
Contributor

@pmai Removed typos and change order of enum Type for traffic sign.

@carsten-kuebler
Copy link
Contributor Author

@CarloVanDriestenBMW and @pmai please review additional extensions to traffic signs and road markings. Please consider:

  1. "generic road marking symbols" like bicycle are currently missing in enum RoadMarkings.Type. We could reuse painted_traffic_sign not only for the image (237 StVO) but also for the generic symbol (Sinnbilder §39 Abs. 7 StVO).
  2. We add a string attribute for textual road markings (e.g. BUS). If OSI will not use strings as attributes, we could proceed similar as 1) --> type generic text symbol. (Drawback: We are limited to textual road markings that are represented as a traffic sign. -> no destinations like M for Munich, but destinations are also missing in OSI traffic signs).

@carsten-kuebler
Copy link
Contributor Author

@CarloVanDriestenBMW and @pmai
3. (re)using painted traffic sign's enum also for generic symbols give the possibility to reduce road marking's enum type. (If we do this, all variations of traffic sign 531 StVO should be added?!)

@ghost ghost added this to the v3.0.0 milestone Feb 2, 2018
@ghost ghost added FeatureRequest Proposals which enhance the interface or add additional features. HelpWanted I have tried my best - please help me! labels Feb 2, 2018
@ghost
Copy link

ghost commented Feb 9, 2018

// Additional value associated with the traffic sign, e.g. value of the speed limit.
// Field need not be set if traffic sign type does not require it. Speed (limits) should be given in km/h.
optional double value = 4;

are there countries which have traffic signs in two different units? IF SO: We have to add a unit field?

@ghost
Copy link

ghost commented Feb 9, 2018

@carsten-kuebler some thoughts:

to 1) I would say YES - we are defining the logical information of traffic signs here, therefore the logical information is stored in the trafficSign.Type for (237 StVO) but also for the generic symbol (Sinnbilder §39 Abs. 7 StVO). The information can then be referenced in optional TrafficSign.Type painted_sign_type = 4;
IMO:
It would be easier if the roadmarkings.types could be merged with the trafficSign.types resulting in a single enum describing the logical information.

to 2) I would leave the string for the texts painted as roadMarkings as proposed in your PR.

to 3) as written before: The goal would be a single enum for logical types and additional flags and infos describing that logical information when painted on the street.
Taxi as "string" "just the image of a taxi" or the official "taxi allowed traffic sign"

could you do that?

@pmai pmai force-pushed the Extended-Traffic-Signs branch from 600c82e to 8b8c45a Compare February 13, 2018 00:21
@pmai
Copy link
Contributor

pmai commented Feb 13, 2018

I've fixed the merge conflict and rebased the changes to make them mergeable, should build correctly now (also with the auto-merging PR build). @CarloVanDriestenBMW if that fits your ideas, we can directly merge this.

@carsten-kuebler
Copy link
Contributor Author

@pmai thankyou

@ghost
Copy link

ghost commented Feb 13, 2018

@carsten-kuebler Is the list completely checked against the different STVOs? Some have references and some not?

@carsten-kuebler
Copy link
Contributor Author

@CarloVanDriestenBMW The list is checked against StVO. We add all traffic signs, which were missing in respect to begin/end signs, similar groups of signs, etc. There are a lot of more traffic signs in StVO. All this traffic signs have their StVO reference.

We add also "traffic signs" which exist only in one way in StVO, e.g. "no u-turn right" (StVO has only "no u-turn left").

We also add "traffic signs" which do not exist in StVO, e.g. "u-turn left" which exists only as road marking. (There is a "u-turn" traffic sign in South Korea.

These additional traffic signs have no reference to StVO (because they are not used in germany).

@ghost
Copy link

ghost commented Feb 13, 2018

@pmai and @carsten-kuebler isn't that something for the "DetectedTrafficSign" ?

optional Vector3d view_normal = 6;

There is no viewing angle in a global view or?

@carsten-kuebler
Copy link
Contributor Author

@CarloVanDriestenBMW it is necessary to define what is left or right from the signs point of view ("no parking" sign with an "arrow to the left" and the traffic sign on the left/right side of the lane) and to define if you can see the traffic sign (in a construction area if the sign is turned way "out of order" or if you can see the sign if the road turns).

@pmai pmai mentioned this pull request Feb 14, 2018
@pmai pmai force-pushed the Extended-Traffic-Signs branch from 8b8c45a to 3d15399 Compare February 15, 2018 09:25
@ghost ghost mentioned this pull request Feb 20, 2018
@ghost
Copy link

ghost commented Feb 23, 2018

@carsten-kuebler : look at #138

@carsten-kuebler
Copy link
Contributor Author

@CarloVanDriestenBMW I made a new PR for issue #138. It adresses an exact definition of the BaseStationary::orientation and how to apply the road markings and traffic signs.
This PR adresses still the TYPE definition of the road marking.

@ghost
Copy link

ghost commented Mar 1, 2018

@carsten-kuebler I got the feeling that we will have some conflicts here ^^

@carsten-kuebler
Copy link
Contributor Author

@CarloVanDriestenBMW resolved :-)

@carsten-kuebler
Copy link
Contributor Author

@CarloVanDriestenBMW merge conflict resolved....

@pmai
Copy link
Contributor

pmai commented Mar 2, 2018

I'll again rebase this branch onto master to cleanup the history prior to merging.

@carsten-kuebler
Copy link
Contributor Author

@pmai it should already been cleaned up, isn't it?

@pmai
Copy link
Contributor

pmai commented Mar 2, 2018

@carsten-kuebler a lot of the old commits that were replaced by the rebase I did earlier have resurfaced (after a rebase a git reset --hard origin/branchname on the branch is needed to get rid of the old commits in ones own repository), plus lots of intermediate merges with master by carlo somewhat mess up the history and we probably don't want to pull that into master... So content-wise it should be ok, but the history is really messed up.

Removed typos and change order of enum Type for traffic sign.
Extend road markings with painted symbols (similar to traffic sign icons).
Extend road markings in form of characters (e.g. BUS) - First use of string attribute in OSI!
Reorder enum RoadMarking.Type and add painted_traffic_sign and text.
carsten-kuebler and others added 2 commits March 2, 2018 10:06
Reuse traffic signs as road markings. Arrows, stop lines, crosswalks,
etc. are monochrome symbols of traffic signs.
Missing CR added.
@pmai pmai force-pushed the Extended-Traffic-Signs branch from aad7f99 to 833450f Compare March 2, 2018 09:41
@pmai
Copy link
Contributor

pmai commented Mar 2, 2018

Ok, I've re-rebased against master, and lost the CI-problem-induced Type Renaming for Road Markings again. This should be the correct content now (without superceded changes and other unrelated changes due to master merges). Please do a git fetch, git checkout Extended-Traffic-Signs followed by git reset --hard origin/Extended-Traffic-Signs if you need to work further on this branch.

@CarloVanDriestenBMW please add positive review if this should be merged now...

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.

positive

@pmai pmai merged commit 2b5610e into master Mar 2, 2018
@pmai pmai deleted the Extended-Traffic-Signs branch March 27, 2018 12:16
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. HelpWanted I have tried my best - please help me!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants