Skip to content

Commit

Permalink
Address CI build warnings (#2742)
Browse files Browse the repository at this point in the history
This PR addresses warnings which have crept into the CI build logs.
Worst case is probably esp32 with IDF 5.2 (GCC 13.2) producing 350 warnings, now 65.

**General fixes**

- Remove deprecated calls to command processing library from samples
- Fix initialisation order and type names: still some instances of e.g. `uint8` which should be `uint8_t`.
- Discovered component samples aren't built for Host in CI! Fixed this - note build time extended accordingly.
- Simplify nested conditionals https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f56-avoid-unnecessary-condition-nesting

**Fix compiler warnings/errors**

Replaced a few more instances of printf with streaming operators, simpler and type-safe.
Fixed `Overloaded virtual` warnings.

**Note**: See remaining warnings for NimBLE (submodule) which need attention.

**Graphics library fixes**

- Fix control rendering: Should use their own bounds for positioning, set button font
- Fix const parameters
- use unique_ptr for getGlyph calls
- Use `std::make_unique` and assignment instead of reset()
- Avoid printf
- Demote `reinterpret_cast` to `static_cast` where appropriate
- Apply coding style to samples
- Fix compiler warnings

**Disable esp32 warning about RWX segments**

Check introduced with linker (LD, binutils 2.39) from IDF 5.2 onwards
This specifically refers to IRAM, which we do actually need to be writeable and executable.
(IDF link code disables this warning also.)
  • Loading branch information
mikee47 authored Mar 25, 2024
1 parent dc7d3fa commit 629678a
Show file tree
Hide file tree
Showing 44 changed files with 113 additions and 92 deletions.
5 changes: 5 additions & 0 deletions Sming/Arch/Esp32/app.mk
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ LDFLAGS += \
-nostdlib \
-Wl,-static

ifeq ($(IDF_VERSION),v5.2)
LDFLAGS += \
-Wl,--no-warn-rwx-segments
endif

ifdef IDF_TARGET_ARCH_RISCV
LDFLAGS += \
-nostartfiles \
Expand Down
4 changes: 2 additions & 2 deletions Sming/Arch/Esp8266/Components/esp8266/include/ets_sys.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ typedef void (*ets_isr_t)(void*);
void ets_intr_lock(void);
void ets_intr_unlock(void);
void ets_isr_attach(int i, ets_isr_t func, void* arg);
void ets_isr_mask(uint32 mask);
void ets_isr_unmask(uint32 unmask);
void ets_isr_mask(uint32_t mask);
void ets_isr_unmask(uint32_t unmask);

void NmiTimSetFunc(void (*func)(void));

Expand Down
6 changes: 3 additions & 3 deletions Sming/Arch/Esp8266/Components/esp8266/include/sdk/mem.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
extern "C" {
#endif

void* pvPortMalloc(size_t xWantedSize, const char* file, uint32 line);
void* pvPortMalloc(size_t xWantedSize, const char* file, uint32_t line);
void* pvPortCalloc(size_t count, size_t size, const char*, unsigned);
void* pvPortZalloc(size_t xWantedSize, const char* file, uint32 line);
void vPortFree(void* ptr, const char* file, uint32 line);
void* pvPortZalloc(size_t xWantedSize, const char* file, uint32_t line);
void vPortFree(void* ptr, const char* file, uint32_t line);
void* vPortMalloc(size_t xWantedSize);
void pvPortFree(void* ptr);

Expand Down
4 changes: 2 additions & 2 deletions Sming/Arch/Esp8266/Components/spi_flash/flashmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ uint32_t flashmem_write_internal( const void *from, uint32_t toaddr, uint32_t si
{
assert(IS_ALIGNED(from) && IS_ALIGNED(toaddr) && IS_ALIGNED(size));

SpiFlashOpResult r = spi_flash_write(toaddr, (uint32*)from, size);
SpiFlashOpResult r = spi_flash_write(toaddr, (uint32_t*)from, size);
if(SPI_FLASH_RESULT_OK == r)
return size;
else{
Expand All @@ -280,7 +280,7 @@ uint32_t flashmem_read_internal( void *to, uint32_t fromaddr, uint32_t size )
{
assert(IS_ALIGNED(to) && IS_ALIGNED(fromaddr) && IS_ALIGNED(size));

SpiFlashOpResult r = spi_flash_read(fromaddr, (uint32*)to, size);
SpiFlashOpResult r = spi_flash_read(fromaddr, (uint32_t*)to, size);
if(SPI_FLASH_RESULT_OK == r)
return size;
else{
Expand Down
4 changes: 2 additions & 2 deletions Sming/Arch/Esp8266/Core/Digital.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void digitalWrite(uint16_t pin, uint8_t val)
if(pin != 16)
GPIO_REG_WRITE((((val != LOW) ? GPIO_OUT_W1TS_ADDRESS : GPIO_OUT_W1TC_ADDRESS)), (1 << pin));
else
WRITE_PERI_REG(RTC_GPIO_OUT, (READ_PERI_REG(RTC_GPIO_OUT) & (uint32)0xfffffffe) | (uint32)(val & 1));
WRITE_PERI_REG(RTC_GPIO_OUT, (READ_PERI_REG(RTC_GPIO_OUT) & (uint32_t)0xfffffffe) | (uint32_t)(val & 1));

//GPIO_OUTPUT_SET(pin, (val ? 0xFF : 00));
}
Expand All @@ -115,7 +115,7 @@ uint8_t digitalRead(uint16_t pin)
if(pin != 16)
return ((GPIO_REG_READ(GPIO_IN_ADDRESS) >> pin) & 1);
else
return (uint8)(READ_PERI_REG(RTC_GPIO_IN_DATA) & 1);
return (uint8_t)(READ_PERI_REG(RTC_GPIO_IN_DATA) & 1);

//return GPIO_INPUT_GET(pin);
}
Expand Down
6 changes: 3 additions & 3 deletions Sming/Arch/Esp8266/Core/Interrupts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ static void interruptDelegateCallback(uint32_t interruptNumber)
* @param intr_mask Interrupt mask
* @param arg pointer to array of arguments
*/
static void IRAM_ATTR interruptHandler(uint32 intr_mask, void* arg)
static void IRAM_ATTR interruptHandler(uint32_t intr_mask, void* arg)
{
bool processed;

do {
uint32 gpioStatus = GPIO_REG_READ(GPIO_STATUS_ADDRESS);
uint32_t gpioStatus = GPIO_REG_READ(GPIO_STATUS_ADDRESS);
processed = false;
for(uint8 i = 0; i < MAX_INTERRUPTS; i++) {
for(uint8_t i = 0; i < MAX_INTERRUPTS; i++) {
if(!bitRead(gpioStatus, i)) {
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions Sming/Arch/Esp8266/Platform/RTC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ bool RtcClass::setRtcSeconds(uint32_t seconds)

void updateTime(RtcData& data)
{
uint32 rtc_cycles;
uint32 cal, cal1, cal2;
uint32_t rtc_cycles;
uint32_t cal, cal1, cal2;
cal1 = system_rtc_clock_cali_proc();
__asm__ __volatile__("memw" : : : "memory"); // Just for fun
cal2 = system_rtc_clock_cali_proc();
Expand Down
4 changes: 2 additions & 2 deletions Sming/Arch/Host/Components/esp_hal/clk.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ static uint32_t get_ccount(uint64_t nanos)
return ccount;
}

bool system_update_cpu_freq(uint8 freq)
bool system_update_cpu_freq(uint8_t freq)
{
if(freq == cpu_frequency) {
return true;
Expand All @@ -46,7 +46,7 @@ uint8_t ets_get_cpu_frequency(void)
return cpu_frequency;
}

uint8 system_get_cpu_freq(void)
uint8_t system_get_cpu_freq(void)
{
return ets_get_cpu_frequency();
}
Expand Down
2 changes: 1 addition & 1 deletion Sming/Arch/Host/Components/esp_hal/include/esp_clk.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ extern "C" {
#define SYS_CPU_80MHZ 80
#define SYS_CPU_160MHZ 160

bool system_update_cpu_freq(uint8 freq);
bool system_update_cpu_freq(uint8_t freq);
uint8_t ets_get_cpu_frequency(void);
uint8_t system_get_cpu_freq(void);

Expand Down
2 changes: 1 addition & 1 deletion Sming/Arch/Host/Components/esp_hal/include/esp_sleep.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void wifi_fpm_auto_sleep_set_in_null_mode(uint8_t req);

/* GPIO */

void wifi_enable_gpio_wakeup(uint32 i, GPIO_INT_TYPE intr_status);
void wifi_enable_gpio_wakeup(uint32_t i, GPIO_INT_TYPE intr_status);
void wifi_disable_gpio_wakeup(void);

/* These aren't defined in the RTOS SDK */
Expand Down
2 changes: 1 addition & 1 deletion Sming/Arch/Host/Components/esp_hal/include/esp_system.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ void os_delay_us(uint32_t us);

const char* system_get_sdk_version(void);

uint32 system_get_chip_id(void);
uint32_t system_get_chip_id(void);

bool system_rtc_mem_read(uint8_t src_addr, void* des_addr, uint16_t load_size);
bool system_rtc_mem_write(uint8_t des_addr, const void* src_addr, uint16_t save_size);
Expand Down
2 changes: 1 addition & 1 deletion Sming/Arch/Host/Components/esp_hal/sleep.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void wifi_fpm_auto_sleep_set_in_null_mode(uint8_t req)

/* GPIO */

void wifi_enable_gpio_wakeup(uint32 i, GPIO_INT_TYPE intr_status)
void wifi_enable_gpio_wakeup(uint32_t i, GPIO_INT_TYPE intr_status)
{
}

Expand Down
2 changes: 1 addition & 1 deletion Sming/Arch/Host/Components/esp_hal/system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ const char* system_get_sdk_version(void)
return version_string;
}

uint32 system_get_chip_id(void)
uint32_t system_get_chip_id(void)
{
return 0xC001BEAF;
}
Expand Down
2 changes: 1 addition & 1 deletion Sming/Arch/Host/Components/spi_flash/flashmem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ uint8_t flashmem_get_size_type()
return flashmem_get_info().size;
}

uint32 spi_flash_get_id(void)
uint32_t spi_flash_get_id(void)
{
return 0xFA1E0008;
}
Expand Down
2 changes: 1 addition & 1 deletion Sming/Arch/Host/Core/Digital.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

// Wemos D1 mini has pin 16
#define PIN_MAX 16
static uint8 pinModes[PIN_MAX + 1];
static uint8_t pinModes[PIN_MAX + 1];

DigitalHooks defaultHooks;
static DigitalHooks* activeHooks = &defaultHooks;
Expand Down
1 change: 1 addition & 0 deletions Sming/Arch/Host/Tools/ci/build.run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ if [[ $CHECK_SCA -eq 1 ]]; then
"$DIR/coverity-scan.sh"
else
$MAKE_PARALLEL samples DEBUG_VERBOSE_LEVEL=3
$MAKE_PARALLEL component-samples DEBUG_VERBOSE_LEVEL=3
fi

# Build and run tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ AccessPointClass& WifiAccessPoint = accessPoint;

void AccessPointImpl::enable(bool enabled, bool save)
{
uint8 mode;
uint8_t mode;
if(save) {
mode = wifi_get_opmode_default() & ~SOFTAP_MODE;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class BssInfoImpl : public BssInfo

void StationImpl::enable(bool enabled, bool save)
{
uint8 mode;
uint8_t mode;
if(save) {
mode = wifi_get_opmode_default() & ~STATION_MODE;
} else {
Expand Down
30 changes: 16 additions & 14 deletions Sming/Components/Network/Arch/Host/Platform/StationImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,24 +161,26 @@ bool StationImpl::isEnabled() const
bool StationImpl::config(const Config& cfg)
{
for(auto& ap : apInfoList) {
if(cfg.ssid == ap.ssid) {
if(ap.authMode != AUTH_OPEN) {
if(cfg.password != ap.pwd) {
debug_w("Bad password for '%s'", cfg.ssid.c_str());
return false;
}
}
if(cfg.ssid != ap.ssid) {
continue;
}

currentAp = &ap;
if(cfg.save) {
savedAp = &ap;
if(ap.authMode != AUTH_OPEN) {
if(cfg.password != ap.pwd) {
debug_w("Bad password for '%s'", cfg.ssid.c_str());
return false;
}
}

debug_i("Connected to SSID '%s'", cfg.ssid.c_str());

autoConnect = cfg.autoConnectOnStartup;
return true;
currentAp = &ap;
if(cfg.save) {
savedAp = &ap;
}

debug_i("Connected to SSID '%s'", cfg.ssid.c_str());

autoConnect = cfg.autoConnectOnStartup;
return true;
}

debug_w("SSID '%s' not found", cfg.ssid.c_str());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class HttpServerConnection : public HttpConnection
}
}

using TcpClient::send;
using HttpConnection::send;

void setUpgradeCallback(HttpServerProtocolUpgradeCallback callback)
{
Expand All @@ -86,6 +86,11 @@ class HttpServerConnection : public HttpConnection
}

protected:
bool send(HttpRequest* request) override
{
return HttpConnection::send(request);
}

// HTTP parser methods

int onMessageBegin(http_parser* parser) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include <Libraries/ArduCAM/ov2640_regs.h>

ArduCamCommand::ArduCamCommand(ArduCAM& CAM, CommandProcessing::Handler& commandHandler)
: myCAM(CAM), commandHandler(&commandHandler), imgSize(OV2640_320x240), imgType(JPEG)
: myCAM(CAM), commandHandler(&commandHandler), imgType(JPEG), imgSize(OV2640_320x240)
{
debug_d("ArduCamCommand Instantiating");
}
Expand Down Expand Up @@ -155,7 +155,7 @@ void ArduCamCommand::setType(const String& type)
setFormat(type == "BMP" ? BMP : JPEG);
}

void ArduCamCommand::setFormat(uint8 type)
void ArduCamCommand::setFormat(uint8_t type)
{
if(type == BMP) {
myCAM.set_format(BMP);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ class ArduCamCommand
bool status = true;
ArduCAM myCAM;
CommandProcessing::Handler* commandHandler{nullptr};
uint8 imgType;
uint8 imgSize;
uint8_t imgType;
uint8_t imgSize;

void processSetCommands(String commandLine, ReadWriteStream& commandOutput);

void setFormat(uint8 type);
void setFormat(uint8_t type);
void showSettings(ReadWriteStream& commandOutput);

const char* getImageType();
Expand Down
2 changes: 1 addition & 1 deletion Sming/Libraries/IOControl
2 changes: 1 addition & 1 deletion Sming/Libraries/LittleFS
2 changes: 1 addition & 1 deletion Sming/Libraries/MDNS
3 changes: 1 addition & 2 deletions Sming/Libraries/MFRC522/MFRC522.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ void MFRC522::setSPIConfig() {
} // End setSPIConfig()


void ICACHE_FLASH_ATTR
MFRC522::setControlPins(byte csPin,byte pdPin) {
void MFRC522::setControlPins(byte csPin,byte pdPin) {
_chipSelectPin = csPin;
_resetPowerDownPin = pdPin;

Expand Down
2 changes: 1 addition & 1 deletion Sming/Libraries/MFRC522/MFRC522.h
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ class MFRC522 {
// Functions for manipulating the MFRC522
/////////////////////////////////////////////////////////////////////////////////////
void PCD_Init();
void ICACHE_FLASH_ATTR setControlPins(byte csPin,byte pdPin);
void setControlPins(byte csPin,byte pdPin);
void PCD_Reset();
void PCD_AntennaOn();
void PCD_AntennaOff();
Expand Down
2 changes: 1 addition & 1 deletion Sming/Libraries/OneWire/OneWire.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class OneWire
OneWire(uint8_t pin);

void begin();
void begin(uint8 pinOneWire);
void begin(uint8_t pinOneWire);

// Perform a 1-Wire reset cycle. Returns 1 if a device responds
// with a presence pulse. Returns 0 if there is no device or the
Expand Down
2 changes: 0 additions & 2 deletions Sming/Libraries/OtaUpgrade/OtaUpgrade/BasicStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
#include <FlashString/Array.hpp>
#include <debug_progmem.h>

extern "C" uint32 user_rf_cal_sector_set(void);

namespace OtaUpgrade
{
#ifndef ENABLE_OTA_DOWNGRADE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ void otaUpdate()

void showInfo()
{
Serial.printf(_F("\r\nSDK: v%s\r\n"), system_get_sdk_version());
Serial.printf(_F("Free Heap: %d\r\n"), system_get_free_heap_size());
Serial.printf(_F("CPU Frequency: %d MHz\r\n"), system_get_cpu_freq());
Serial.printf(_F("System Chip ID: %x\r\n"), system_get_chip_id());
Serial << endl << _F("SDK: v") << system_get_sdk_version() << endl;
Serial << _F("Free Heap: ") << system_get_free_heap_size() << endl;
Serial << _F("CPU Frequency: ") << system_get_cpu_freq() << _F(" MHz") << endl;
Serial << _F("System Chip ID: ") << String(system_get_chip_id(), HEX, 8) << endl;

int total = 0;
for(auto part : OtaManager.getBootPartitions()) {
Expand Down
14 changes: 7 additions & 7 deletions Sming/Libraries/Servo/Servo.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ class Servo

private:
ServoChannel* channels[maxChannels] = {0};
unsigned channelCount = 0; ///< Number of active channels
HardwareTimer hardwareTimer; ///< Handles generation of output signals
Frame frames[2]; ///< Contains the active and next frames
uint8 activeFrameIndex = 0; ///< Frame being used by ISR
uint8 activeSlot = 0; ///< Slot being output by ISR
uint8_t nextFrameIndex = 0; ///< Frame to use when active frame has completed
SimpleTimer updateTimer; ///< If necessary, updates are pended
unsigned channelCount = 0; ///< Number of active channels
HardwareTimer hardwareTimer; ///< Handles generation of output signals
Frame frames[2]; ///< Contains the active and next frames
uint8_t activeFrameIndex = 0; ///< Frame being used by ISR
uint8_t activeSlot = 0; ///< Slot being output by ISR
uint8_t nextFrameIndex = 0; ///< Frame to use when active frame has completed
SimpleTimer updateTimer; ///< If necessary, updates are pended
};

extern Servo servo; ///< global instance of the servo object
Expand Down
2 changes: 1 addition & 1 deletion Sming/Libraries/UPnP-Schema
Loading

0 comments on commit 629678a

Please sign in to comment.