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

Code shuffling for output modes #26

Merged

Conversation

therealprof
Copy link
Collaborator

These changes shuffle the code a around a little bit to make room for the addition of new operating modes with alternative APIs.

Signed-off-by: Daniel Egger <daniel@eggers-club.de>
Signed-off-by: Daniel Egger <daniel@eggers-club.de>
Modes can be switched by using functions, e.g. into_egfx() so the
Egfx examples where changed to use that.

Signed-off-by: Daniel Egger <daniel@eggers-club.de>
@therealprof therealprof force-pushed the features/splitoutput branch from f02a68c to ab2bc32 Compare March 29, 2018 11:34
@jamwaffles
Copy link
Collaborator

Can I close this in favour of #27?

@therealprof
Copy link
Collaborator Author

As mentioned in #27 I'd like to keep things separate so they can be worked on independently. #27 is really just a proof of concept for #26. From my POV we just need to agree on a name for egfx to land this.

@jamwaffles
Copy link
Collaborator

jamwaffles commented Mar 31, 2018

As of right now, I see the mode names and functions being this:

  • NoMode: Expose an interface to init the display and send a &[u8] to the display. The simplest driver possbile really.
  • GraphicsMode: Use embedded_graphics to twiddle pixels and draw primitives, images and text. This uses a framebuffer and also encompasses the functionality of a PixelMode or BitmapMode by providing the pixel twiddling methods. The library doesn't support pixel twiddling yet, but I can see a common case where someone wants to draw complex things as well as set pixels, so having these two "modes" in one place makes sense to me.
  • CharacterMode: Bufferless, character-only mode.

I'd like to see a trait called DisplayMode that has the methods reset, init and set_rotation. These are common to all modes so should always be available.

@therealprof
Copy link
Collaborator Author

NoMode: Expose an interface to init the display and send a &[u8] to the display. The simplest driver possbile really.

You mean as in expose just the raw commands? I'd suggest I start with the renaming and then we'll take it from there.

@therealprof
Copy link
Collaborator Author

Instead of NoMode we could also call it RawMode.

@jamwaffles jamwaffles mentioned this pull request Mar 31, 2018
@jamwaffles
Copy link
Collaborator

You mean as in expose just the raw commands? I'd suggest I start with the renaming and then we'll take it from there.

Erm, I don't think so. Not for now at least; I think commands need a bit more thought. It would essentially just expose send_data() along with the DisplayMode trait fns.

Yeah RawMode sounds better actually. NoMode implies the driver won't do anything by default which isn't true.

Signed-off-by: Daniel Egger <daniel@eggers-club.de>
@therealprof therealprof requested a review from jamwaffles March 31, 2018 10:24
@therealprof
Copy link
Collaborator Author

@jamwaffles Is this good to go? I'd like to work on the stuff depending on these changes. I can also squash the changes if you'd like to get rid of the intermediate state.

@jamwaffles
Copy link
Collaborator

I'm liking where this is going, but two last things:

  1. Is it worth adding a send_raw_bytes() (or some other named) method to RawMode? We're exposing it from the crate but it isn't very useful to the user right now. Could just not make it public?
  2. I'd really like to see a trait that implements init() and set_rotation() (and maybe get_dimensions() if that makes sense for all the graphics modes you can think of). What do you think?

@therealprof
Copy link
Collaborator Author

I agree we should have that. I just don't want to overload this one with too much stuff and would rather have this as a dedicated follow up. At the moment this is blocking the CharacterMode implementation and a number of consolidation and cleanup changes so I'd rather have this completed ASAP so we're not restricted with the order of the followup changes.

@jamwaffles jamwaffles merged commit bbc29eb into rust-embedded-community:master Mar 31, 2018
@jamwaffles
Copy link
Collaborator

Okiedoke, LGTM then :). Sorry for holding onto it.

@therealprof therealprof deleted the features/splitoutput branch April 1, 2018 13:42
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