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

Revise system headers / enforce c11 #1695

Merged
merged 7 commits into from
May 21, 2019

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented May 21, 2019

Build C files within modules to c11 standard (previously defaulted to gnu90)

Standardise data types, e.g. use uint32_t not uint32

  • Arduino libraries use a mixture so still need to maintain definitions for those

Tidy and simplify c_types.h

  • Rename espinc/c_types_compatible.h as c_types.h
  • Revise to use stdint and stdbool definitions
  • Move attribute definitions and macros into esp_attr.h
  • Put common attribute definitions into sming_attr.h

user_config.h shouldn't contain system definitions

  • Move system definitions from user_config.h into esp_systemapi.h (exclude LWIP)
  • Don't #include user_config.h from esp_systemapi.h
  • Move missing memory API definitions out of esp_systemapi.h and into mem.h

Simplify Library/sample code using <esp_systemapi.h> instead of assortment of low-level headers

Add WEAK_ATTR macro to allow selective weak symbol support

  • Not a C/C++ standard feature and not consistently applied on all platforms, so avoid where possible.

mikee47 added 7 commits May 21, 2019 06:56
* Use `uint32_t` in preference to `uint32`, etc.
* Arduino libraries use a mixture so still need to maintain definitions for those
* Rename `espinc/c_types_compatible.h` as `c_types.h`, revise to use `stdint` and `stdbool` definitions
* Place location for `c_types.h` and `mem.h` ahead of SDK version in makefiles
* Move system definitions from `user_config.h` into `esp_systemapi.h` (exclude LWIP)
* Don't #include `user_config.h` from `esp_systemapi.h`
* Move missing memory API definitions out of `esp_systemapi.h` and into `mem.h`
* Simplify other headers
* Not a C/C++ standard feature and not consistently applied on all platforms, so avoid where possible.
@slaff slaff added this to the 3.9.0 milestone May 21, 2019
@mikee47 mikee47 changed the title Enforce c11 standard and regularise data types Revise system headers / enforce c11 May 21, 2019
@@ -21,8 +21,7 @@ diff -Nuar a/replacements/libc.c b/replacements/libc.c
+ Modified 03 April 2015 by Markus Sattler
+ */
+
+#include <stdint.h>
+#include "espinc/c_types_compatible.h"
+#include <c_types.h>
+#include <stdarg.h>
+
+extern int ets_putc(int);
Copy link
Contributor

@slaff slaff May 21, 2019

Choose a reason for hiding this comment

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

In this PR make dist-clean did not clean the axtls directory and the new patch was not applied cleanly. Any idea why is that happening?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Try make dist-clean ENABLE_SSL=1

Copy link
Contributor Author

@mikee47 mikee47 May 21, 2019

Choose a reason for hiding this comment

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

We have these optional custom libraries which get built:

pwm_open
mainmm
lwip_full
lwip_open
lwip2
axtls
mqttc

Behaviour is that the library only gets cleaned if the relevant flags are defined, ditto for submodule updating. It would be better of course if they all got cleaned regardless. I can look at this in another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better of course if they all got cleaned regardless. I can look at this in another PR.

Absolutely. It will also match the current default behaviour.

@slaff slaff merged commit b68169b into SmingHub:develop May 21, 2019
@slaff slaff removed the 3 - Review label May 21, 2019
@mikee47 mikee47 deleted the fix/standardise_data_types branch May 21, 2019 13:10
@slaff slaff mentioned this pull request Sep 28, 2019
4 tasks
mikee47 added a commit to mikee47/Sming that referenced this pull request Nov 21, 2021
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.
mikee47 added a commit to mikee47/Sming that referenced this pull request Nov 21, 2021
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.
mikee47 added a commit to mikee47/Sming that referenced this pull request Nov 22, 2021
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.
slaff pushed a commit that referenced this pull request Nov 23, 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.
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