Fixes to facilitate building directly on ESP-IDF#147
Fixes to facilitate building directly on ESP-IDF#147kbx81 wants to merge 1 commit intomikalhart:masterfrom
Conversation
|
As this library isnt really supported much anymore, would it be better having your changes as a different tinyGPS library? If it aint broke, lets not fix it./break it. TINY GPS has been working fine for a ling time, and in reality, very few would use the changes you may want, unless you are happy fixing any errors. I have seen this happen with other libraries. Make a new version. TinyGPSPP-new or whatever |
|
Sorry @svdrummer, I work in open source full-time and would much rather contribute back to the source than "take the money and run." There are already many, many forks of this project all attempting to fix the same thing and I couldn't find one that worked. ESPHome was already using this fork and we need a tiny bit more from it; so, it was "broke", I did fix it and I'd much rather contribute my work back to the source than maintain my own version. As it's now 2025, GPS is not exactly new and isn't changing much; I couldn't find a GPS library that's been updated more recently than 2-3 years ago. Since the rest of the world is clearly advancing and wanting new things from old projects, what exactly is wrong with updating them a little bit to fit in? |
|
AdaFruit has many recent updates. For ESPHome, other than time, what would you want a GPS functions for.? other than time? If you just want time, I can write something for you in C++. I have made changes to a few GPS library's in the past 18 months, as there have been changes to the NMEA0183 format, and NEO is slowly changing to its own HEX format. Then you have deliberate error injections to contend with, other hardware/software decoding. |
|
ESPHome currently supports reporting latitude, longitude, speed, course, altitude, number of visible satellites and HDOP. Yes, the most common use is for time but we also have a number of users who have mobile applications and do indeed use/consume more than just time. If I just wanted to grab time from input sentences, yes, I also know how to implement this myself, but I see no reason to reinvent the wheel yet again. The changes in this PR:
I must also add that I see no reason why this library, given what it does, should be specifically tied to Arduino -- I have to argue that breaking that specific dependency is indeed a good thing. |
|
Fair enough. Valid points. Well thought out. You seem to know your stuff. A lot of people just want changes because they don't have the skill to add a bit of code, which ultimately slows down and makes the library bloated. Arduino is way behind as a wrapper for Espressif's compiler. |
This PR implements patches to permit building ESPHome configurations with this library when using ESP-IDF (instead of Arduino) as the framework. This results in smaller binaries, which is particularly important due to the notable size increase associated with Arduino 3.x. Here's the results from our build tests for the original ESP32:
Arduino:
IDF:
Some of these changes were in #69 but it appears that they were not merged correctly and/or additional changes were made after merging them, at least based on what I see in c093432
I've created additional (ESPHome) tests to verify these changes work when built with ESPHome configurations using both Arduino and IDF; see esphome/esphome#9728