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

remove dependency on the global Serial object #1214

Closed
wants to merge 1 commit into from

Conversation

craiglink
Copy link
Contributor

remove dependency on the global Serial object which was only used in OLEDDisplay to output a deprecated message.

PR to original OLEDDisplay repo has been requested as well: https://github.com/ThingPulse/esp8266-oled-ssd1306/pull/408/files

Reduces RAM and Memory usage

[env:wifi] // before
RAM: [=== ] 27.3% (used 89400 bytes from 327680 bytes)
Flash: [========= ] 91.1% (used 1791125 bytes from 1966080 bytes)

[env:wifi] // after
RAM: [=== ] 27.2% (used 89072 bytes from 327680 bytes)
Flash: [========= ] 90.9% (used 1786609 bytes from 1966080 bytes)

in OLEDDisplay to output a deprecated message.

PR to original OLEDDisplay repo has been requested as well

Reduces RAM and Memory usage

[env:wifi] // before
RAM:   [===       ]  27.3% (used 89400 bytes from 327680 bytes)
Flash: [========= ]  91.1% (used 1791125 bytes from 1966080 bytes)

[env:wifi] // after
RAM:   [===       ]  27.2% (used 89072 bytes from 327680 bytes)
Flash: [========= ]  90.9% (used 1786609 bytes from 1966080 bytes)
@MitchBradley
Copy link
Collaborator

Let's wait to see if they take the PR. I prefer not to use private repos unless they provide a compelling advantage. In this case, eliminating the global Serial saves some memory but not enough to get excited about.

@MitchBradley
Copy link
Collaborator

What I would like is a version of the oled lib that does not use String. That would be worth a private repo.

@bdring
Copy link
Owner

bdring commented May 6, 2024

I would not be sad if the internally coded OLED was deprecated.

@MitchBradley
Copy link
Collaborator

I was thinking the same thing but we have sold hardware that depends on it and both of my production machines use oled to report their connection status. I don't want to give that up.

@MitchBradley
Copy link
Collaborator

I took a swag at eliminating String from the lib. It took about 10 minutes

@MitchBradley
Copy link
Collaborator

The easiest solution would be to just pin the library version to 4.4.1, the version before the change that introduced the global Serial dependency. The changes from 4.4.1 to 4.5.0 look irrelevant to us. We can declare that our built-in OLED support is not going to be improved going forward and it is what it is.

MitchBradley added a commit that referenced this pull request May 6, 2024
This is an alternative solution to the problem addressed by #1214
@robercy
Copy link

robercy commented May 6, 2024

I would not be sad if the internally coded OLED was deprecated.

Hi, with fluid dial on desk, this is a good idea.

@MitchBradley
Copy link
Collaborator

FluidDial cannot connect until after the WiFi connection attempt completes (or fails). I have some systems where OLED shows the status of the WiFi connection attempt, which is useful information when you are waiting to see if FluidDial is going to connect.

MitchBradley added a commit that referenced this pull request May 6, 2024
This is an alternative solution to the problem addressed by #1214
@MitchBradley
Copy link
Collaborator

Superseded by #1215

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

4 participants