search

sudomesh / disaster-radio

A (paused) work-in-progress long-range, low-bandwidth wireless disaster recovery mesh network powered by the sun.
https://disaster.radio
1.06k stars 107 forks source link

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

Closed paidforby closed 4 years ago

paidforby commented 4 years ago

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.

marcidy commented 4 years ago

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 commented 4 years ago

heltecv2oled e.g. this is what I see

paidforby commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

paidforby commented 4 years ago

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.