thorrak / tiltbridge

Tilt Hydrometer to WiFi Bridge
http://www.tiltbridge.com/
Other
61 stars 27 forks source link

Support M5StickC display #185

Closed ricnewton closed 3 years ago

ricnewton commented 3 years ago

Add support for M5Stick-C display. Pin espressif/arduino-esp32 version for now, it seems like the arduino core for the ESP32 has been recently updated on github and is now incompatible with our libraries, pin it to before the update for now.

TaplistIOTeam commented 3 years ago

@ricnewton anything possibly special about your setup/local environment? I have not been able to get the display to light up, so I'm wondering what could be different..

I've done a clean build and flash from your tree as follows:

$ pio run -t clean
$ pio run -e m5stickc --target upload
ricnewton commented 3 years ago

@TaplistIOTeam shouldn't have anything special (I used the platformio vscode plugin to upload but should not make a difference). If you have the Arduino IDE maybe try https://github.com/m5stack/M5StickC/tree/master/examples/Basics/Display first maybe, check that works?

I'm also on Windows, can't think that should matter though.

ricnewton commented 3 years ago

@TaplistIOTeam here is a quick platform io project I lashed up to test out the display, you could try that as well: M5StickC-play.zip

mik3y commented 3 years ago

Thanks for that! Sample project worked on first shot:

image

I guess we can rule out a hardware problem. I'll try wiping spiffs & see if any change.

ricnewton commented 3 years ago

Looks good, something strange going on then. I'd stick some logging in bridge_lcd::init, make sure it's going in the LCD_TFT_M5STICKC part, or maybe change tft->fillScreen(TFT_BLACK); to tft->fillScreen(TFT_BLUE); just to see that it works.

TaplistIOTeam commented 3 years ago

Success! I ended up blowing away the entire .pio/ directory - a "cleaner" clean, I guess, and it finally worked.

What's strange is I saw the object code being recompiled, and the elf re-linked, every time I'd make changes in bridge_lcd.cpp, so I'm quite surprised that this did the trick. Perhaps some sort of subtle change in dependencies, I'm not sure.

Looks great and works great now, thanks!

image

ricnewton commented 3 years ago

Brilliant. Still confused why you didn't see the same compile error I did without pinning the version of framework-arduinoespressif32, but great that it works. I normally do a git clean -xfd when building clean so .pio gets wiped out anyway :).

lbussy commented 3 years ago

I didn't know about that flag - I generally delete it from the UI. Great tip!

Sounds like you guys are close to a working device. Have a look at Tilt-Sim to emulate Tilts for testing.

ricnewton commented 3 years ago

@lbussy Looks like a useful project, will give it a try and see what it looks like.

I know the purple colour looks too dark to me, I switch that colour to Magenta locally on mine, but maybe out of scope for this :).

lbussy commented 3 years ago

Let me know when you've got it where you want it and I'll pull it in a branch here and give it a go.

TaplistIOTeam commented 3 years ago

Still confused why you didn't see the same compile error I did without pinning the version of framework-arduinoespressif32, but great that it works.

Good news, in a way: I am now able to reproduce that error. I'm getting to understand how platformio works and specifically how to caches/vendors third-party library. I blew away ~/.platformio which seems to be where it caches third-party libraries. This caused a next build to reinstall espressif:

$ pio run -e m5stickc
[..]
Tool Manager: Installing git+https://github.com/espressif/arduino-esp32.git
git version 2.31.0
Cloning into '/Users/mikey/.platformio/.cache/tmp/pkg-installing-gtg5bobh'...
remote: Enumerating objects: 3626, done.
remote: Counting objects: 100% (3626/3626), done.
remote: Compressing objects: 100% (2645/2645), done.
remote: Total 3626 (delta 888), reused 2204 (delta 537), pack-reused 0
Receiving objects: 100% (3626/3626), 78.39 MiB | 6.94 MiB/s, done.
Resolving deltas: 100% (888/888), done.
Updating files: 100% (4354/4354), done.
Tool Manager: framework-arduinoespressif32 @ 0.0.0+sha.371f382 has been installed!
[..]
Archiving .pio/build/m5stickc/lib0c7/libUpdate.a
Indexing .pio/build/m5stickc/lib0c7/libUpdate.a
.pio/libdeps/m5stickc/LCBUrl/src/LCBUrl.cpp: In member function 'IPAddress LCBUrl::getIP(const char*)':
.pio/libdeps/m5stickc/LCBUrl/src/LCBUrl.cpp:634:53: error: cannot convert 'ip4_addr*' to 'esp_ip4_addr_t* {aka esp_ip4_addr*}' for argument '3' to 'esp_err_t mdns_query_a(const char*, uint32_t, esp_ip4_addr_t*)'
         esp_err_t err = mdns_query_a(hn, 2000, &addr);
                                                     ^
At global scope:
cc1plus: warning: unrecognized command line option '-Wno-frame-address'
*** [.pio/build/m5stickc/lib603/LCBUrl/LCBUrl.cpp.o] Error 1
[..]

So I suggest you restore your pinning. Sorry for creating any confusion here. I'm going to open a separate PR to enable Github Actions to build pull requests, as a potential solution for this. It looks straightforward to enable, and would be able to give us a reference clean build for every pull request. (It could even be used to generate release artifacts, but I'll let others decide if that's interesting).

lbussy commented 3 years ago

.pio/libdeps/m5stickc/LCBUrl/src/LCBUrl.cpp:634:53: error: cannot convert 'ip4_addr' to 'esp_ip4_addr_t {aka esp_ip4_addr}' for argument '3' to 'esp_err_t mdns_query_a(const char, uint32_t, esp_ip4_addr_t*)' esp_err_t err = mdns_query_a(hn, 2000, &addr);

That's in my Lib actually. I thought I had that fixed (i.e. it looks like you have an old version of the lib.)

lbussy commented 3 years ago

That's the IP address one. The other one looks like a known issue:

https://github.com/espressif/esp-idf/issues/3591

ricnewton commented 3 years ago

There are errors in 2 libraries LIBUrl.cpp and WifiManager. The change that broke it was 7 days ago, LibUrl was last updated 25 days ago as far as I can see.

I'm minded to leave things as is as the breaking changes are not really related to the m5stick stuff, maybe it should be fixed separately?

lbussy commented 3 years ago

I'll have a look here in a little bit. I'm still re-wiring everything from moving onto my new standing desk. :)

ricnewton commented 3 years ago

No worries, it's easy enough to work around for now, looks like espressif dumped a breaking change, I guess that's the danger when we are following master

TaplistIOTeam commented 3 years ago

Yeah, I think it's probably a good general idea to pin external dependencies, where they exist, to an upstream release tag or hash, for the sake of build reproducibility & avoiding issues like this.

lbussy commented 3 years ago

Well, we try to do that when we get to a releasable version. ;)

lbussy commented 3 years ago

@ricnewton I am going to merge this into the m5stick branch and check it out unless you think you need to noodle on it some more?

ricnewton commented 3 years ago

Well, I've just had a play with tilt slim, very useful. I'm happy with the look myself, so unless @TaplistIOTeam has any objections I'd say merge :).

TaplistIOTeam commented 3 years ago

Created the action here: #187

Well, I've just had a play with tilt slim, very useful. I'm happy with the look myself, so unless @TaplistIOTeam has any objections I'd say merge :).

LGTM too!

lbussy commented 3 years ago

I'll let Thorrak have a look at the actions change. I'll merge this in for now.

ricnewton commented 3 years ago

Thanks!

lbussy commented 3 years ago

Seems to compile fine here. I'm going to take a moment and slice up this power-only cable and then I'll be back :)

ricnewton commented 3 years ago

Looks like you might have to delete your ~/.platformio directory to reproduce the compiler error. I can raise a pull request to put the pin work around in for now if that helps