Skip to content

[WIP] Merge dev into master#6

Open
ldgrp wants to merge 48 commits intomasterfrom
dev
Open

[WIP] Merge dev into master#6
ldgrp wants to merge 48 commits intomasterfrom
dev

Conversation

@ldgrp
Copy link
Copy Markdown
Contributor

@ldgrp ldgrp commented Mar 14, 2019

Work in progress. Opening this pull request to set some goals.

Still need to work on the following:

  • Decouple route logic from the hardware.
  • Clean up intermediary.py.
  • Improve protocol design.
  • Automate scraping. (This may already be implemented and/or unnecessary)
  • Document, document, document!
  • Add some tests.
  • Squash commits

@ldgrp ldgrp requested a review from katrinafyi March 14, 2019 22:12
Comment thread intermediary.py
return name.replace(' Line', '').replace(' line', '').split(' - ')

def update_state(self, interval):
def update_state(self, interval: int):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Interval is actually unused. Was going to use it to compute percentage, but that's done in the server-side now.

Comment thread intermediary.py
strip = dest_to_strip[s[1]]
strip_state = self.strip_states[strip]
# print(t)
train_destination = dest_to_strip[s[1]]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

train_destination actually isn't the train's destination. It's the 2-tuple (Colour, Direction).

Comment thread intermediary.py
if len(trip_stops[t.trip_id]) > len(self.used_strips[strip]):
print(f' WARNING: trip {t.trip_id} has more stops than LEDS on {strip}.')
continue
# if len(trip_stops[t.trip_id]) > len(self.line_strip_mapping[train_destination]):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why did you comment this out?

Comment thread intermediary.py
cmd = message.build_flash_command(pin, s_state.colour.value)
else:
print('No command matched.')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks much better

Comment thread message.py


class MessageType(enum.Enum):
OFF = enum.auto()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of using enum.auto(), you can use the protocol's number values, then use these when building the string instead of hardcoding it further down.

Comment thread message.py
return b'0,%d,0,0;' % id


def build_on_command(id: int, color: int) -> bytes:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure about the protocol, but add percent as an optional param to build_on, maybe?

Comment thread message.py
def build_flash_command(id: int, color: int) -> bytes:
return b'2,%d,%d,0;' % (id, color)


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What exactly is the difference between fade and flash?

Is fade merely on, but with opacity?

Comment thread colour.py
@@ -0,0 +1,41 @@
class RGBColour:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

__eq__ and __hash__ could be added. Will help if we ever need to use these as dict keys.

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.

2 participants