Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actuated signals (WIP) #298

Closed
wants to merge 19 commits into from

Conversation

khuston
Copy link
Contributor

@khuston khuston commented Aug 27, 2020

Work in progress -- not ready for review

To Do

  • Refactor signal color so it is a state boolean associated with each TurnGroup Movement rather than hard-coded time at draw time.
  • Add actuation/call state and trigger when vehicle/pedestrian is first ready to turn.
  • Encapsulate stage.protected_groups in anticipation of Phase. Replace usage of stage.protected_groups.iter() with a get_protected_groups method that returns an iterator.
  • Introduce Phase as collection of Movement, which for now just has one Movement. For get_protected_groups, chain together the Movement iterators of each Phase.
  • Create SignalView which provides an immutable view of the TrafficSignal/TrafficSignalState for clients like other actors and for drawing.
  • Implement logic for light changes on actuated signal.
  • Testing
  • Remove PhaseType if no longer useful.
  • Cleanup
  • Make parameters settable in UI

There is some temporary overlap in responsibility because the
Adaptive phase type requires knowledge of the intersection state for
its waiting list, and I don't want to bring that into the new traffic
signal logic. Trying to respect Law of Demeter with the new code.

There may be something that approximates this closely enough so I can
take handling the Adaptive phase into the responsibility of the
traffic_signals module without needing this extra information.
@dabreegster
Copy link
Collaborator

Something that may simplify your life -- PhaseType::Adaptive is probably a misnomer, I think I meant "actuated". It was meant to be a proof-of-concept for #91 -- the policy itself (repeat the entire phase duration if a single protected turn is waiting at the end) is comically unrealistic. IIUC, the extension time in your proposal subsumes it. So I think you can rip out PhaseType::Adaptive and you don't need ControlType::Original?

@michaelkirk
Copy link
Collaborator

michaelkirk commented Sep 2, 2020 via email

@dabreegster dabreegster reopened this Sep 2, 2020
@dabreegster
Copy link
Collaborator

Hah wow, weird that a PR can close another PR and not just an issue. Always been a bit confused that they share namespaces.

@dabreegster
Copy link
Collaborator

I'm closing this PR, since @BruceBrown has implemented variable stages that pretty much capture all of this behavior. But thanks so much for working on this, and please reopen if there's any future work you have in mind.

@dabreegster dabreegster closed this Mar 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants