- Notifications
You must be signed in to change notification settings - Fork13.3k
Minor release EspSoftwareSerial 8.2.0 requires ghostl as stand-alone lib dependency#8915
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Conversation
mcspr commentedApr 23, 2023
Just use braces to init? #include<atomic>structFoo { std::atomic<bool> bar {false };}; gcc8 stdlib should be ok with that |
dok-net commentedApr 23, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@mcspr Too short, I don't have a clue what you are talking about. Anyway, since 8.0.3 has passed the Arduino library release checks and all now, is this a blocking issue you are referring me to? Again, please let me know what you are talking about, I'll gladly put in the sources, ready for the next release :-) |
dok-net commentedJul 2, 2023
@mcspr This release 8.1.0 is ready for ESP32 IDF 5.1 and fixes a few other compilation problems as well. Please merge at your discretion. |
f2d4047 tobc25eb9Comparedok-net commentedJan 5, 2024
Some careful consideration by the core team is required here: the new ghostl library apparently does get pulled in just fine by the Arduino library dependency framework, but this is breaking with the traditional model of the ESP8266 core that has all required libs as copies or git submodules. |
mcspr commentedJan 5, 2024
Arduino library dependency framework would pick things up during interactive session, true. Not the case here, library must be present in search path. Note that if you'd do that, now you have 2 libs to sync to here :> And Core suddenly has an external dependency for no reason. Bundling is not an option? |
dok-net commentedJan 6, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
"Bundling" like previously, or are you suggesting something I haven't thought of? Anyway: are the only place where the EspSoftwareSerial lib is used directly. |
dok-net commentedJan 6, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I've checked the aforementioned sketches. The all appear to use EspSoftwareSerial in order to have two UARTs available, one for logging. IIRC the second UART of ESP8266 is TX only, but perfectly suited to that task. Therefore, somebody could rewrite the sketches just fine and free the EspSoftwareSerial hard dependency. Everyone needing a real duplex SW UART can use the Arduino library manager way. |
mcspr commentedJan 18, 2024
Agreed, examples should prefer real uart here.
Just put it inside of the lib folder as previously, but with an added namespace. Meaning #include<circular_queue.h> Becomes this #include<ghostl/circular_queue.h> (but, I think here it must also drop any mention of ghostl from library .json and .properties)
True, but so is the case for WiFi libs or anything coming from the overly-simplified library manager. IDE / CLI that causes a lot of headache when using multiple development platforms. Specifically the fact that $SKETCHLIB/libraries is overriding Core lib(s) for some reason. PIO actually has it solved with project-private libs folders, but it is more of an Arduino-adjacent solution and an altogether different tool. I'm still on the 'keep it here' side, so repo has a version of swserial that definitely works. Whether it is up-to-date is whole other question. |
dok-net commentedJan 20, 2024
I don't understand, going backward to
and including ghostl as a copy in EspSoftwareSerial is not an option now - just to state it categorically once more. Is there anything I am missing with this PR, putting ghostl side-by-side in the submodules list? This works and finally solves one on the update headaches. If you're puzzled about the project ownership - I am the original author of ghostl, AND I am the acting maintainer of EspSoftwareSerial, too - so there is conflict of interest or potential hazard that one or the other would be handled differently with regards to ESP8266 Arduino core's needs. Are considering merging this PR at all? |
… multiple copy&paste inclusions in various repos into an own repo to use as separate library dependency
…no library via the lib manager
mcspr commentedJan 20, 2024
Fixing CI might be a good start? Can't merge if 'Squash and merge' button does not work. Line 203 in47327e8
skip_sketch, followed byskip_ino must skip impossible-to-build sketches, which includes ghostl coroutine examples. Either by explicitly mentioning them, or creatingSKETCH_DIR/.test.skipTo reiterate - the idea is to have '#include' path contain all of the required files in one step,just for the Core distribution. And all while inside of the single lib directory, possibly using a separate branch that have these already merged. If you don't think that is viable, ok, we'll ship both. Namespacing of library includes would be helpful regardless? Seems like a commonplace practice for more-or-less complex libs. |
dok-net commentedJan 20, 2024
Ah, CI, right. I'm currently using a platform.local.txt: |
mcspr commentedJan 20, 2024
See#8916 and#8990. Main concerns are with new -std=... build defaults applied to every lib and sketch besides our Core files, (sometimes useless) new warnings, and also incomplete support and its unknown quality in gcc10 branch compared to gcc11 and gcc12. Maybe next version, not really feeling adventurous just yet |
dok-net commentedJan 21, 2024
About translating c++ namespace into directory stucture ... good point... I'm working that out. Bear with me. |
mcspr commentedJan 21, 2024
OK to merge this? |
Uh oh!
There was an error while loading.Please reload this page.
@mcspr I'm sorry, my fault.