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

Update to FBInk v1.21.0 #9

Merged
merged 11 commits into from
Feb 8, 2020
Merged

Update to FBInk v1.21.0 #9

merged 11 commits into from
Feb 8, 2020

Conversation

NiLuJe
Copy link
Collaborator

@NiLuJe NiLuJe commented Feb 2, 2020

The usual deal ;).

Except I attempted to implement GetLastRect, so you might want to double-check that (c.f., the last two commits), because I basically have no idea what I'm doing :D.

@shermp
Copy link
Owner

shermp commented Feb 2, 2020

Hey, thanks @NiLuJe

As you can probably see from the activity of the repo, I haven't exactly been doing much in this area for a while.

Looking at some of the changes you've made, I may have to look at bumping the major version to v2.x.x, as it looks like there is some API breakage.

Will have a look at the changes in more detail, but first glance looks alright.

gofbink/gofbink.go Show resolved Hide resolved
gofbink/gofbink.go Outdated Show resolved Hide resolved
Co-Authored-By: Sherman Perry <14854761+shermp@users.noreply.github.com>
@NiLuJe
Copy link
Collaborator Author

NiLuJe commented Feb 2, 2020

Fun fact: As far as fbink_refresh is concerned, it actually restores the "original" signature of the function (at least in C land) ^^.

@shermp
Copy link
Owner

shermp commented Feb 2, 2020

I will need to up the major version tag to avoid go get -u updating to a version that then might break the users code, since in Go land, the tools assume minor/patch updates don't break the existing API.

I was just thinking, have you thought about breaking fbink into separate libraries? Like a base library that plots pixels, and manipulates the framebuffer and eink refreshes, and one that handles printing text/images etc?

@NiLuJe
Copy link
Collaborator Author

NiLuJe commented Feb 2, 2020

Possibly, vaguely, one day, maybe (i.e., if there's ever a v2 API) :D.

But I'm terrible at modularization ^^.

@NiLuJe
Copy link
Collaborator Author

NiLuJe commented Feb 2, 2020

No, but, seriously, the main issue is the globals: I kind of need at least bits and bobs of it everywhere.
Which means the right thing to do would probably? be to chuck all of that in a "Context" struct setup at init, that is just passed by ref everywhere.
Which means you can then move/reorganize FBInkConfig & FBInkOTConfig & co. in there.
That makes the very few things that have to enforce specific FBInkConfig flags slightly trickier, though...
And it also deprecates to a large extent FBInkState/fbink_get_state.
Which means that's basically a full refactor of the API, hence my earlier "v2" comment ^^.

In the meantime, the current workaround is the modular buildsystem, f.g., KFMon links to a 50KB minimal build, which essenttially boils down to pixel plotting, refresh, and IBM fixed-cell rendering.

@shermp
Copy link
Owner

shermp commented Feb 2, 2020

Fair enough on the v2 API. I have to admit I personally find the single FBConfig struct has gotten so large with so many options that it's gotten rather unwieldy to work with.

But this PR probably isn't the place to be discussing hypothetical new API approaches, so I'll leave it there. We can always have a chat on the FBInk repo at some point if you so desire.

@NiLuJe
Copy link
Collaborator Author

NiLuJe commented Feb 2, 2020 via email

@shermp
Copy link
Owner

shermp commented Feb 6, 2020

BTW, I'm finally rebuilding my linux dev VM that I accidentally nuked a while back, so I'll be able to test this in the near future.

(It was a mistake of me thinking I was setting up a new VM, when I accidentally booted into an existing VM...)

Currently setting up koxtoolchain. That will probably take nearly an hour.

This keeps the linter happy, and helps with auto generated docs
@shermp
Copy link
Owner

shermp commented Feb 8, 2020

Fixed a couple of compilation errors.

Also, although not part of this particular PR, I've added and fixed some comments on exported types and methods. Comments before exported types/methods are not quite as ad-hoc as they may appear. The comment should begin with the exported type/method name. This all ties into how the documentation is generated. And keeps the linter happy...

@shermp
Copy link
Owner

shermp commented Feb 8, 2020

Well it runs on my H2O, so merge time it is :)

@shermp shermp merged commit 8fde6db into shermp:master Feb 8, 2020
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