Port Samsung display driver

[pkheav]

No description provided.

[kimburgess]

Really nice start! Take a dig through some of these comments, then hit me up with anything that doesn't make sense.

We'll likely go through a few rounds of this. Please don't take this as my being overly critical, this is just a good opportunity to discuss and start building a common coding style for these drivers under crystal.

The main things to remember when porting across is that clean, highly solid implementations > feature parity. They definitely will not be a line-for-line port and these re-writes are a good opportunity to ensure we create a nice, maintainable code base.

[kimburgess]

Looking nice! A couple of minor things to still neaten up.

For the specs: rebase this branch from the current master. This will pull in the latest dependencies and restore compatibility with the current state of the driver framework and crystal compiler. You’ll need to force push that back here, but no issues doing that while this is pre-merge.

[pkheav]

Hey @kimburgess , just re requested your review. I've addressed all your changes except for this one #19 (comment)

[kimburgess]

Only a couple of super small point for discussion here. Let's sort them, check all the spec pass then gtg!

[kimburgess]

Looking good - confirming specs are green here.

Before me merge as one last thing can you add some specs that cover power and switch_to.

[kimburgess]

Nice one. Things should work in this state.

Before merge I'm opening up a discussion here with @stakach and @caspiano. There's some behaviour in this driver that's a carry over from the old one: both the power state and input are forced to specific targets based on the last applied state. If these change from an external source, the target states will be reapplied when this is detected.

IMHO this is something that we should make a rule to disallow (or at least strongly discourage) at the device driver level. Doing so will keep the drivers simpler so they purely provide control and an accurate view of the current state. These other behaviours can then be abstracted into logic drivers and applied where needed. It also removes some of potential failure modes of having duplicate module instances, other systems, or humans interacting with a physical device competing against each other. Thoughts?

[kimburgess]

@stakach bump on the query above.

[stakach]

The way it works in the ruby version, I believe, is that the state is forced if selected by the driver.
Once in a stable state then it no longer forces the input and external changes are OK

The reason for doing this is that the display ignores commands, or is busy when powering on etc
and should go to the requested state as soon as it can

So I think the behaviour is desirable and generally external changes won't be overridden