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

Fix system headers, review and simplify #2431

Merged
merged 32 commits into from
Nov 23, 2021

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented Nov 22, 2021

This PR removes some long-deprecated code and build logic from the framework,
and fixes some low-level (system) header file inconsistencies between architectures.

user_config.h

This header was removed from samples in #1695.
It's a hangover from the Espressif SDK applications and doesn't fit very well into the framework structure.

The file has been removed from all framework code but one copy left with a deprecation warning.
Note added to upgrade docs.

System header files

Each architecture has a set of system header files, located here:

  • Arch/Esp8266/Components/esp8266
  • Arch/Esp32/Components/esp32
  • Arch/Host/Components/esp_hal
  • Arch/Rp2040/Components/rp2040

In particular the ESP8266 SDK headers are problematic so have been moved into the esp8266 Component:

  • Remove sntp.h as can just use lwip/sntp.h
  • Apply coding style, fix type definitions for consistency
  • Integrate 'missing' SDK definitions - these are no longer scattered around on an ad-hoc basis
  • Split out user_interface.h into esp_system.h and esp_wifi.h
  • LWIP_OPEN_SRC is always true so can remove SDK version of ip_addr.h
  • Move BIT, TRUE, FALSE, LOCAL macros into c_types.h

For other architectures:

  • Move os_random and os_get_random into esp_system (get rid of esp_libc)
  • Remove system_set_os_print and ets_install_putc1 (not used)

Remove pre-SDK 3 support

There was an issue with small flash sizes but this now seems to be resolved.
Tested with Basic_Blink sample and 256K, 512K flash size, no problem.
Now using only libraries from the SDK: all header/linker files are in framework.

Simplifies build system and code. SDK_BASE definition is no longer publicised.

SDK submodule renamed from ESP8266_NONOS_SDK to sdk.

lwip

  • Use single lwip_includes.h file, relocated to Network Component, rather than one-per-arch
  • Fix esp_open_lwip
  • Fix lwip2, but as it's forked can use that instead of patching

Other simplifications

  • esp8266 esp_cplusplus.h header not necessary, single function used only by startup code
  • Remove new-pwm submodule, replace with patched source instead
  • Provide better esp32 system_get_time implementation
  • Remove deprecated headers (including SmingCore/SmingCore.h)
  • Move SERIAL_BAUD_RATE, ESP8266_EX and ESP32_EX definitions into makefiles

Build system

  • Fix issue with missing Component -clean targets.
    e.g. make lwip2-clean will always fail because ENABLE_CUSTOM_LWIP option never gets set, so lwip2-clean target never gets created

  • Fix component-samples build target, should honour COMPONENT_SOC setting

  • Fix component-samples-clean build target, broken

Integration tests

Add component-samples to esp32 build, requires only a few simple library fixes to build all variants.

e.g. `make lwip2-clean` will always fail because ENABLE_CUSTOM_LWIP option never gets set, so `lwip2-clean` target never gets created
Was removed from applications in SmingHub#1695.
It's a hangover from C esp8266 SDK applications and makes maintenace difficult because it contains so much stuff.
Instead of working around them, we should have a consistent set.
The SDK is no longer in development so these are very unlikely to change.
Tested with Basic_Blink sample and 256K, 512K flash size, no problem.
Uses only libraries from SDK submodule.
Don't bother patching, use fork
- Honour COMPONENT_SOC setting
- `component-samples-clean` just plain broken
- Simple library fixes
@slaff slaff added this to the 4.5.0 milestone Nov 22, 2021
Note that not all samples are built for Windows - perhaps they should be?
Add Basic_Templates anyway.
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