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

[WIP] Add basic simulator support #29

Merged
merged 5 commits into from
May 5, 2018

Conversation

byronwasti
Copy link
Contributor

This isn't a completed feature yet, but I think its a good baseline to get us started.

I'm wondering about the functionality of this feature: how are people going to use it and what form does it need to be in?

Currently it is implemented as a submodule in a workspace, but I'm not convinced that is the best form for it to take.

@jamwaffles jamwaffles self-requested a review May 3, 2018 17:34
Copy link
Member

@jamwaffles jamwaffles left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for doing this. Even works on Windows Subsystem for Linux!

image

Can you fix the build though? I think just installing libsdl2-dev should be fine if you want to build simulator, but I'm more interested in making sure embedded-graphics continues to build and push docs ok. Just adding -p embedded-graphics to the cargo commands should be enough.

Currently it is implemented as a submodule in a workspace, but I'm not convinced that is the best form for it to take.

I'm on the fence about this one. It might make more sense as a separate repo, but on the flipside it's useful to distribute a simulator with the library to test things without needing a hardware display. I tinker with this lib at work where I don't have a display to work on for example. It may also be useful in the future as part of the CI infra. Let's leave it where it is for now. It can easily be split out later :)

@jamwaffles jamwaffles merged commit e59bdf6 into embedded-graphics:master May 5, 2018
@jamwaffles
Copy link
Member

Sorry for letting this sit, didn't realise you'd pushed fixes. This should be a good base to build on even if there's a better place for it to live in the future. Thanks!

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