Skip to content

Conversation

@carsten-kuebler
Copy link
Contributor

This PR addresses issue #70 .

Review and reformating of documentation.

Unifying enum names.

Change Traffic Lights from semantic representation (light box) to syntactic representation (light bulbs)

@ghost ghost requested a review from ppannusch-bmw March 15, 2018 17:29
@ghost ghost added this to the v3.0.0 milestone Mar 15, 2018
@ghost ghost added the FeatureRequest Proposals which enhance the interface or add additional features. label Mar 15, 2018
// The base parameters of the traffic light.
//
// \c BaseStationary::orientation x-axis is view normal of the traffic
// sign's icon.

Choose a reason for hiding this comment

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

traffic light, not sign ;)

// geometrical arrangement) and the state \c TrafficLight::mode is
// \c MODE_OFF then \c TrafficLight::color could remain unchanged.
// If The traffic light displays different images in different colors,
// then \c TrafficLight::mode

Choose a reason for hiding this comment

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

something missing to finish that sentence?

// This traffic light counter in percent.
//
ICON_COUNTDOWN_PERCENT = 22;
}

Choose a reason for hiding this comment

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

What about Bus, Bus_and_Tram (yes they exist :)

@carsten-kuebler
Copy link
Contributor Author

@ppannusch Thx. I add the buses and remove the typos.

@pmai pmai force-pushed the update/traffic_light branch from 9cf26e2 to 2b8dc18 Compare March 19, 2018 09:34
@pmai
Copy link
Contributor

pmai commented Mar 19, 2018

@carsten-kuebler I've rebased onto master and fixed (as far as I can tell) the spurious conflicts (this branch always won), so please git checkout update/traffic_light && git fetch && git reset --hard origin/update/traffic_light prior to further changes...

@ghost
Copy link

ghost commented Mar 20, 2018

@carsten-kuebler I accidently pressed update branch... I will review and you can do as P.Mai suggests on the prior commit, work in the review and push afterwards.

@pmai pmai force-pushed the update/traffic_light branch from e12d219 to 929012f Compare March 20, 2018 14:15
@pmai
Copy link
Contributor

pmai commented Mar 20, 2018

@carsten-kuebler, @CarloVanDriestenBMW rebased and conflict-free, should be mergeable 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.

I think we should add the supplementary sign "baseStationary" as discussed in this PR

// A list of candidates for (a) possible supplementary sign(s) as estimated by the sensor.
//
// A list of candidates for (a) possible supplementary sign(s) as estimated
// by the sensor.
Copy link

Choose a reason for hiding this comment

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

spaces?

@pmai pmai self-assigned this Mar 21, 2018
Carsten Kuebler and others added 4 commits March 21, 2018 10:01
Review and reformating of documentation.

Unifying enum names.

Change Traffic Lights from semantic representation (light box) to syntactic representation (light bulbs)
TrafficLight defines one "bulb" and not one "box" e.g. red, yello green. Now every "bulb" is a separate message.
Remove Typos
Add buses to traffic light icons.
@pmai pmai force-pushed the update/traffic_light branch from 929012f to dd19945 Compare March 21, 2018 09:03
@pmai
Copy link
Contributor

pmai commented Mar 21, 2018

@CarloVanDriestenBMW Fixed up Doxygen and rebased to remove conflicts from new master.

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.

@carsten-kuebler I will merge this one and the other changes to the traffic lights will be done in a seperate PR in order to keep the master as up to date a s possible

@ghost ghost dismissed ppannusch-bmw’s stale review March 21, 2018 16:20

On vacation. I checked it

@ghost ghost merged commit f762dbf into master Mar 21, 2018
@pmai pmai deleted the update/traffic_light branch March 27, 2018 12:16
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.

4 participants