Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pull Core/Network out into separate Component #2316

Merged
merged 7 commits into from
Apr 27, 2021

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented Apr 25, 2021

The DISABLE_WIFI setting was introduced as an experimental feature for Esp8266 to reduce image sizes. This PR moves the networking support code into a separate library so it doesn't need to be built for these applications.

This also improves built times for tools using the Host architecture, for example the LittleFS fscopy tool.

The related code for each architecture has also been moved, including related units from Platform.

Documentation has been revised and a few minor fixes included.

@mikee47 mikee47 changed the title Pull Core/Network out into separate Component [WIP] Pull Core/Network out into separate Component Apr 25, 2021
@mikee47 mikee47 force-pushed the feature/disable-wifi branch 2 times, most recently from bcdc245 to 2b7de24 Compare April 25, 2021 16:05
@mikee47 mikee47 changed the title [WIP] Pull Core/Network out into separate Component Pull Core/Network out into separate Component Apr 26, 2021
@slaff slaff added this to the 4.3.1 milestone Apr 26, 2021
@@ -12,7 +12,7 @@

#include "HttpRequestAuth.h"
#include "HttpRequest.h"
#include "Network/WebHelpers/base64.h"
#include <Data/WebHelpers/base64.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikee47 Please start an upgrade document for this release and add information that Network/WebHelpers/base64.h is now moved to Data/WebHelpers/base64.h.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You happy with this change? We could leave the path as-is... or provide an alias so it doesn't break anything...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or provide an alias so it doesn't break anything

I am ok with the change. It would be great if you can add the old header file pointing to the new one with a deprecation notice for smoother transition. Similar to what we have here: https://github.com/SmingHub/Sming/blob/d375ef74fd11f0796aa4483c0bdaacb4fb755655/Sming/SmingCore/SmingCore.h

@mikee47 mikee47 force-pushed the feature/disable-wifi branch from 183bb01 to ad7a451 Compare April 26, 2021 16:49
@mikee47 mikee47 force-pushed the feature/disable-wifi branch from ad7a451 to 3826133 Compare April 26, 2021 19:26
@slaff slaff removed the 3 - Review label Apr 27, 2021
@slaff slaff merged commit 8b853ff into SmingHub:develop Apr 27, 2021
@mikee47 mikee47 deleted the feature/disable-wifi branch April 29, 2021 15:34
@slaff slaff mentioned this pull request May 5, 2021
5 tasks
slaff pushed a commit that referenced this pull request Sep 27, 2021
The `DISABLE_WIFI` setting was introduced as an experimental feature for Esp8266 to reduce image sizes. This PR moves the networking support code into a separate library so it doesn't need to be built for these applications.

This also improves built times for tools using the Host architecture, for example the LittleFS `fscopy` tool.

The related code for each architecture has also been moved, including related units from `Platform`.

Documentation has been revised and a few minor fixes included.
mikee47 added a commit to mikee47/Sming that referenced this pull request Oct 9, 2021
mikee47 added a commit to mikee47/Sming that referenced this pull request Oct 9, 2021
mikee47 added a commit to mikee47/Sming that referenced this pull request Oct 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants