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

More printing fixes #399

Merged
merged 3 commits into from
Mar 17, 2024
Merged

Conversation

ropg
Copy link
Contributor

@ropg ropg commented Mar 14, 2024

  • setLogBuffer needs to be re-called (and past buffer cleared) every time a setFont happens, as this changes the number of lines and chars. It now does not take arguments anymore and does things internally and is clearer about when it gives up.
  • calculation of shiftUp for when on last line was subtly wrong

* setLogBuffer needs to be re-called (and past buffer cleared) every time a `setFont` happens, as this changes the number of lines and chars. It now does not take arguments anymore and does things internally and is clearer about when it gives up.
* calculation of shiftUp for when on last line was wrong
@marcelstoer
Copy link
Member

It now does not take arguments anymore and does things internally

The the fewer the parameters, the better 👍

However, breaking public API is not something that we take lightly. We could keep the old function in the API for the time being. It could print a deprecation message to serial and delegate to the new function?

@ropg
Copy link
Contributor Author

ropg commented Mar 15, 2024

I'm not so sure I can think of any scenario where the user would want to interact with either drawLogBuffer or setLogBuffer. Maybe we should rename them to something that's private or protected and print deprecation msgs for both?

@ropg
Copy link
Contributor Author

ropg commented Mar 15, 2024

"deprecated: print functionality now handles buffer management automatically" ?

@marcelstoer
Copy link
Member

Whatever we do, we can't remove functions from public API without a major version bump. Hence, I suggest something like

bool OLEDDisplay::setLogBuffer(uint16_t lines, uint16_t chars) {
  Serial.println("[deprecated] Print functionality now handles buffer management automatically. This is a no-op.");
}

setLogBuffer() can then be made private API I guess.

@ropg
Copy link
Contributor Author

ropg commented Mar 16, 2024

It occurred to me that it's maybe cleaner to have cls() and setFont() remove the buffer and then only the next print action re-set it as needed. That way users have the option to free the memory by just issuing cls(). Will make it all so later today.

old versions with arguments deprecated, new versions protected.
@marcelstoer
Copy link
Member

This looks sane, thanks a lot.

@marcelstoer marcelstoer merged commit 2ef5428 into ThingPulse:master Mar 17, 2024
6 checks passed
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.

None yet

2 participants