Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
dok-net wants to merge7 commits intoesp8266:master
base:master
Choose a base branch
Loading
fromdok-net:swserial

Conversation

@dok-net
Copy link
Contributor

@dok-netdok-net commentedApr 23, 2023
edited
Loading

@mcspr I'm sorry, my fault.

@dok-netdok-net changed the titleBug fix EspSoftwareSerial 8.0.3:: Arduino library repo didn't pick up 8.0.2 properBug fix EspSoftwareSerial 8.0.3: Arduino library repo didn't pick up 8.0.2 properApr 23, 2023
@mcspr
Copy link
Collaborator

Just use braces to init?

#include<atomic>structFoo {  std::atomic<bool> bar {false };};

gcc8 stdlib should be ok with that
(~/toolchain-xtensa-esp32@8.4.0+2021r2-patch5/bin/xtensa-esp32-elf-g++ -c atomic.cpp works with the above)

@dok-net
Copy link
ContributorAuthor

dok-net commentedApr 23, 2023
edited
Loading

@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 :-)

mcspr referenced this pull request in plerup/espsoftwareserialApr 23, 2023
@dok-netdok-net marked this pull request as draftJune 18, 2023 07:22
@dok-netdok-net changed the titleBug fix EspSoftwareSerial 8.0.3: Arduino library repo didn't pick up 8.0.2 properMinor release EspSoftwareSerial 8.1.0Jul 2, 2023
@dok-net
Copy link
ContributorAuthor

@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.

@dok-netdok-net marked this pull request as ready for reviewJuly 2, 2023 09:31
@dok-netdok-net marked this pull request as draftJuly 23, 2023 15:41
@dok-netdok-netforce-pushed theswserial branch 2 times, most recently fromf2d4047 tobc25eb9CompareJanuary 4, 2024 22:28
@dok-netdok-net changed the titleMinor release EspSoftwareSerial 8.1.0Minor release EspSoftwareSerial 8.2.0 requires ghostl as stand-alone lib dependencyJan 5, 2024
@dok-net
Copy link
ContributorAuthor

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.

@dok-netdok-net marked this pull request as ready for reviewJanuary 5, 2024 09:38
@mcspr
Copy link
Collaborator

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
Copy link
ContributorAuthor

dok-net commentedJan 6, 2024
edited
Loading

"Bundling" like previously, or are you suggesting something I haven't thought of?

Anyway:

libraries/esp8266/examples/SerialStress/SerialStress.inolibraries/ESP8266WiFi/examples/WiFiTelnetToSerial/WiFiTelnetToSerial.inolibraries/lwIP_PPP/examples/PPPServer/PPPServer.ino

are the only place where the EspSoftwareSerial lib is used directly.
My focus has moved from the ESP8266 to the ESP32 family (some of them have just as few HW UARTs as the ESP8266), like most users seem to have as well. The inclusion as a submodule in ESP8266 core has been a source of pain for everyone using both ESP8266 and ESP32s and me far too long already.
I'm adding the ghostl library as a new submodule in this PR now, but I am not convinced. Perhaps do this and merge once now, then sometime soon remove BOTH submodules and either document or change the few places that use EspSoftwareSerial in the ESP8266 tree, for everyone's benefit.
I don't have much of a preference, except that I will not be able to spend much effort - plus everyone complaining about issues with multiple versions of the libs between ESP8266's included and any Arduino / platformio downloaded ones in ./libraries.

@dok-net
Copy link
ContributorAuthor

dok-net commentedJan 6, 2024
edited
Loading

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
Copy link
Collaborator

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.

Agreed, examples should prefer real uart here.

"Bundling" like previously, or are you suggesting something I haven't thought of?

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)

I don't have much of a preference, except that I will not be able to spend much effort - plus everyone complaining about issues with multiple versions of the libs between ESP8266's included and any Arduino / platformio downloaded ones in ./libraries.

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.
(I can always just cherry-pick the version to sync here if you don't want to :)

@dok-net
Copy link
ContributorAuthor

I don't understand, going backward to

#include <ghostl/circular_queue.h>

and including ghostl as a copy in EspSoftwareSerial is not an option now - just to state it categorically once more.
Especially that now somebody was kind enough to submit ghostl to the platformio library repository.

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?

@mcspr
Copy link
Collaborator

Fixing CI might be a good start? Can't merge if 'Squash and merge' button does not work.

skip=$(skip_sketch"$sketch""$sketchname""$sketchdir""$sketchdirname")

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.skip

To 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.
(btw aren't the even more problems b/c of something else depending on ghostl causing out-of-sync versions yet again?)

Namespacing of library includes would be helpful regardless? Seems like a commonplace practice for more-or-less complex libs.#include <ghostl.hpp> for everything,#include <ghostl/foo.h> for specific stuff. Otherwise this is yet another problem where header names can clash with each other.

@dok-net
Copy link
ContributorAuthor

Ah, CI, right.
Would you consider adding this to the default?

I'm currently using a platform.local.txt:

build.stdcpp_level=-std=gnu++20compiler.cpp.extra_flags=-fcoroutines -std=gnu++20

@mcspr
Copy link
Collaborator

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
Copy link
ContributorAuthor

About translating c++ namespace into directory stucture ... good point... I'm working that out. Bear with me.

@mcspr
Copy link
Collaborator

OK to merge this?

mcspr added a commit to mcspr/esp8266-Arduino that referenced this pull requestFeb 4, 2024
mcspr added a commit to mcspr/esp8266-Arduino that referenced this pull requestFeb 4, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dok-net@mcspr

[8]ページ先頭

©2009-2025 Movatter.jp