search

thorrak / tiltbridge

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

Fix framework-arduinoespressif32 issue #188

Closed ricnewton closed 3 years ago

ricnewton commented 3 years ago

Latest master branch has broken some libraries, pin to just prior to breaking changes for now.

Resolves #189

TaplistIOTeam commented 3 years ago

/me anxiously watches the github actions run 😅

ricnewton commented 3 years ago

All green :)

TaplistIOTeam commented 3 years ago

Nice!!

TaplistIOTeam commented 3 years ago

Tested locally, confirmed works as well.

@lbussy I think this is the last thing necessary to turn devel green again.

lbussy commented 3 years ago

I can merge it, but I don't understand it. :)

@thorrak?

thorrak commented 3 years ago

I can merge it, but I don't understand it. :)

@thorrak?

I mean... The answer is somewhere in here.

Showing 5,236 changed files with 843,750 additions and 340,175 deletions.

Yeeaahhhh. They bumped the underlying framework from 1.0.6 to 2.0. I'm going to vote we merge this commit, stick with the 1.0.x branch for now, and worry about upgrading to 2.0.x much later.

ricnewton commented 3 years ago

@lbussy So commit https://github.com/espressif/arduino-esp32/commit/5502879a5b25e5fff84a7058f448be481c0a1f73 in the arduino-esp32 repo changed a load of apis (including e.g. mdns_query_a) that libraries LCBUrl and WiFiManager need, so a completely clean build will currently fail if arduino-esp32 is post that commit.

It seems like some sort of caching in platform io hides this sometime as @TaplistIOTeam had to remove his .platformio directory in the home directory to be able to reproduce the failure.

ricnewton commented 3 years ago

@lbussy FYI I got LIBUrl working by changing struct ip4_addr addr; to esp_ip4_addr_t addr; in LCBUrl::getIP, but then WifiManager still fails

TaplistIOTeam commented 3 years ago

It seems like some sort of caching in platform io hides this sometime as @TaplistIOTeam had to remove his .platformio directory in the home directory to be able to reproduce the failure.

That's right; as is, the dependency is expressed as "repo @ master"; which of course will have a different value depending on when evaluated. It seems platformio sees a checked-out repo@master and decides that's good enough.

Arguably, it's a good general practice even in the absence of build breakage to pin third-party dependencies to a specific version, so that builds are reproducible and "hermetic": The same checkout from our tree is guaranteed to build against the same total set of sources, and yields the same object code. Further, you have the auditability benefit that dependency changes are always logged explicitly, by a commit/pr.

(You'll see this style of dependency pinning in other ecosystems, eg go's modules & node's package-lockfile.json, etc. It's of course up to you whether to adopt it here.)