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

undefined reference to `vtable for OLEDClient' when compiling sparkfun or heltec board #74

Closed
paidforby opened this issue May 20, 2020 · 7 comments

Comments

@paidforby
Copy link
Contributor

When running the pio run -e sparkfun-lora or pio run -e heltec-v2 with OLED_SDA set, I receive the following linker error,

.pio/build/sparkfun-lora/src/main.ino.cpp.o:(.literal._Z12setupDisplayv+0x4): undefined reference to vtable for OLEDClient'

I tried to figure out the root cause, I think it has something to do with how the OLEDClient object constructor or destructor is declared. I don't have a sparkfun-lora to actually test with, so I just made bandage fix by commenting out OLED_SDA, which disables the OLED display.

Creating this issue as a reminder and a call for help from someone who has a sparkfun-lora or heltec-v2 to test with.

paidforby pushed a commit that referenced this issue May 20, 2020
…tructor for OLED client, add lopy4 to travis build
@marcidy
Copy link
Contributor

marcidy commented Jun 19, 2020

I have the OLED working on heltec-v2, take a look at my fork at https://github.com/marcidy/disaster-radio/tree/heltec-v2-oled for the changes. Pretty minimal syntax stuff.

@marcidy
Copy link
Contributor

marcidy commented Jun 19, 2020

heltecv2oled
e.g. this is what I see

@paidforby
Copy link
Contributor Author

Thanks for testing that. That looks right. I think your changes may cause issues for boards that don't have OLED screens. those #ifdef OLED_SDA guards are there to prevent the OLED code from being compiled for boards that do not have OLED screens. If you open a merge request, I can make the necessary changes and test on some of the other boards I have available.

@marcidy
Copy link
Contributor

marcidy commented Jun 22, 2020

Oh, I removed those just for testing, I'll rebase before making a PR. Initially I worked of the commit in this thread, is it easier for you if I rebase against master?

@marcidy
Copy link
Contributor

marcidy commented Jun 25, 2020

Hm, scratch my above, i went back and added the OLED_SDA directives and it didn't work again.

The actual issue is the way the files are compiled.

The pins aren't defined at the time of compiling since the compiler visits each file separately. So when compiling OLEDClient.cpp, it hadn't defined the pin. When compiling main.ino, it has the pin, so it tries to define the body of the setupDisplay function.

So a quick fix is adding #include "../../config.h" to OLEDClient.h. But this is a bit ugly since that information isn't required to compile the code, the only thing that's needed is the defines.

A different approach would be to define a variable passed through the build process to include various hardware, then guard based on this variable, and adding it to the sections in platformio.ini related to the target boards.

Then you can rely on it being defined for that board that needs it, but the specifics of which pins to use still sit in config.h, and you don't need to include it.

So, rather than relying on OLED_SDA, set a build flag in platformio.ini like USE_OLED.

I'll put a PR for that if you agree that it makes sense.

@marcidy
Copy link
Contributor

marcidy commented Jun 25, 2020

added PR as an example so there's something tangible to look at.

@paidforby
Copy link
Contributor Author

Great stuff.

The pins aren't defined at the time of compiling since the compiler visits each file separately. So when compiling OLEDClient.cpp, it hadn't defined the pin. When compiling main.ino, it has the pin, so it tries to define the body of the setupDisplay function.

That's a sensible explanation, I was having trouble wrapping my head around why it wasn't compiling and had not invested much time in it since I didn't have a heltec to test on.

Adding a build flag is a good solution. Might be worth to make that sort of option available for all the various modules, so folks have more granularity in controlling what is included in their firmware. But that's for another issue.

Thanks for the PR, merging it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants