From b36e5ee396d4f4d3e619b5ab9d7b3b995c6efbd6 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Wed, 16 Oct 2019 23:48:15 +0300 Subject: [PATCH 1/7] test: allow to run a single configuration test, improve commandline handling --- .bandit | 3 + .travis.yml | 2 +- code/scripts/espurna_utils/__init__.py | 21 ++++ code/scripts/espurna_utils/display.py | 12 +- code/scripts/test_build.py | 133 ++++++++++++++++++++ code/test/build/{ => extra}/secure_client.h | 0 code/test_build.sh | 39 ------ travis_script.sh | 2 +- 8 files changed, 168 insertions(+), 44 deletions(-) create mode 100644 .bandit create mode 100755 code/scripts/test_build.py rename code/test/build/{ => extra}/secure_client.h (100%) delete mode 100755 code/test_build.sh diff --git a/.bandit b/.bandit new file mode 100644 index 00000000..eec99a94 --- /dev/null +++ b/.bandit @@ -0,0 +1,3 @@ +# Ignore B404 "Avoid importing subprocess" +# Ignore B603 "Subprocess without shell equals true" +skips: ['B404', 'B603'] diff --git a/.travis.yml b/.travis.yml index e627821a..22c62479 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,7 +26,7 @@ jobs: - stage: Test env: BUILDER_ENV=travis-2_3_0 - env: BUILDER_ENV=travis-latest - - env: BUILDER_ENV=travis-git BUILDER_EXTRA=secure_client + - env: BUILDER_ENV=travis-git BUILDER_EXTRA="-a test/build/extra/secure_client.h" - stage: Release env: BUILDER_THREAD=0 - env: BUILDER_THREAD=1 diff --git a/code/scripts/espurna_utils/__init__.py b/code/scripts/espurna_utils/__init__.py index 62387a3c..174e56aa 100644 --- a/code/scripts/espurna_utils/__init__.py +++ b/code/scripts/espurna_utils/__init__.py @@ -1,3 +1,24 @@ +# coding=utf-8 +# +# Original extra_scripts.py +# Copyright (C) 2016-2019 by Xose Pérez +# +# ldscripts, lwip patching, updated postmortem flags and git support +# Copyright (C) 2019 by Maxim Prokhorov +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + from .checks import check_cppcheck, check_printsize from .float_support import remove_float_support from .ldscripts import ldscripts_inject_libpath diff --git a/code/scripts/espurna_utils/display.py b/code/scripts/espurna_utils/display.py index 0628f94b..0e614b66 100644 --- a/code/scripts/espurna_utils/display.py +++ b/code/scripts/espurna_utils/display.py @@ -1,10 +1,11 @@ from __future__ import print_function +import os import sys -import click class Color(object): + BOLD = "\x1b[1;1m" BLACK = "\x1b[1;30m" RED = "\x1b[1;31m" GREEN = "\x1b[1;32m" @@ -31,8 +32,13 @@ def print_warning(message, color=Color.LIGHT_YELLOW): print(clr(color, message), file=sys.stderr) -def print_filler(fill, color=Color.WHITE, err=False): - width, _ = click.get_terminal_size() +def print_filler(fill, color=Color.WHITE, err=False, width_default=80): + width = width_default + try: + width = int(os.environ["COLUMNS"]) + except (KeyError, ValueError): + pass + if len(fill) > 1: fill = fill[0] diff --git a/code/scripts/test_build.py b/code/scripts/test_build.py new file mode 100755 index 00000000..c65c984a --- /dev/null +++ b/code/scripts/test_build.py @@ -0,0 +1,133 @@ +#!/usr/bin/env python +# +# Copyright (C) 2019 by Maxim Prokhorov +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import print_function + +import time +import glob +import argparse +import atexit +import subprocess +import os +import sys +import datetime +from espurna_utils.display import Color, clr, print_warning + +CUSTOM_HEADER = "espurna/config/custom.h" +if os.path.exists(CUSTOM_HEADER): + raise SystemExit( + clr( + Color.YELLOW, + "{} already exists, please run this script in a git-worktree(1) or a separate directory".format( + CUSTOM_HEADER + ), + ) + ) + + +def try_remove(path): + try: + os.remove(path) + except: # pylint: disable=bare-except + print_warning("Please manually remove the file `{}`".format(path)) + + +atexit.register(try_remove, CUSTOM_HEADER) + + +def main(args): + configurations = [] + if not args.no_default: + configurations = list(glob.glob(args.default_configurations)) + + configurations.extend(x for x in (args.add or [])) + if not configurations: + raise SystemExit(clr(Color.YELLOW, "No configurations selected")) + + print(clr(Color.BOLD, "> Selected configurations:")) + for cfg in configurations: + print(cfg) + if args.list: + return + + if not args.environment: + raise SystemExit(clr(Color.YELLOW, "No environment selected")) + print(clr(Color.BOLD, "> Selected environment: {}".format(args.environment))) + + for cfg in configurations: + print(clr(Color.BOLD, "> Building {}".format(cfg))) + with open(CUSTOM_HEADER, "w") as custom_h: + + def write(line): + sys.stdout.write(line) + custom_h.write(line) + + name, _ = os.path.splitext(cfg) + name = os.path.basename(name) + write('#define MANUFACTURER "TEST_BUILD"\n') + write('#define DEVICE "{}"\n'.format(name.upper())) + with open(cfg, "r") as cfg_file: + for line in cfg_file: + write(line) + + os_env = os.environ.copy() + os_env["PLATFORMIO_SRC_BUILD_FLAGS"] = "-DUSE_CUSTOM_H" + os_env["PLATFORMIO_BUILD_CACHE_DIR"] = "test/pio_cache" + cmd = ["platformio", "run", "-s", "-e", args.environment] + + start = time.time() + subprocess.check_call(cmd, env=os_env) + end = time.time() + print( + clr( + Color.BOLD, + "> {}: {} bytes, {}".format( + cfg, + os.stat( + os.path.join(".pio", "build", args.environment, "firmware.bin") + ).st_size, + datetime.timedelta(seconds=(end - start)), + ), + ) + ) + + +if __name__ == "__main__": + parser = argparse.ArgumentParser() + parser.add_argument( + "-l", "--list", action="store_true", help="List selected configurations" + ) + parser.add_argument( + "-n", + "--no-default", + action="store_true", + help="Do not use default configurations (--default-configurations=...)", + ) + parser.add_argument( + "-a", + "--add", + action="append", + help="Add path to selected configurations (can specify multiple times)", + ) + parser.add_argument("-e", "--environment", help="PIO environment") + parser.add_argument( + "--default-configurations", + default="test/build/*.h", + help="(glob) default configuration headers", + ) + + main(parser.parse_args()) diff --git a/code/test/build/secure_client.h b/code/test/build/extra/secure_client.h similarity index 100% rename from code/test/build/secure_client.h rename to code/test/build/extra/secure_client.h diff --git a/code/test_build.sh b/code/test_build.sh deleted file mode 100755 index 7fef5b8b..00000000 --- a/code/test_build.sh +++ /dev/null @@ -1,39 +0,0 @@ -#!/bin/bash - -set -eu -o pipefail - -CUSTOM_HEADER="espurna/config/custom.h" -TARGET_ENVIRONMENT=${1:?"pio env name"} -shift 1 - -CONFIGURATIONS=( - basic - sensor - emon - light_my92xx - light_dimmer - nondefault -) - -if [ $# > 0 ] ; then - CONFIGURATIONS=("${CONFIGURATIONS[@]}" "$@") -fi - -trap 'rm -f ${CUSTOM_HEADER}' EXIT - -for cfg in "${CONFIGURATIONS[@]}" ; do - echo "travis_fold:start:build_${cfg}" - echo "- building ${cfg}" - - printf "#define MANUFACTURER \"%s\"\n" "TEST_BUILD" \ - | tee ${CUSTOM_HEADER} - printf "#define DEVICE \"%s\"\n" "${cfg^^}" \ - | tee --append ${CUSTOM_HEADER} - tee --append ${CUSTOM_HEADER} < "test/build/${cfg}.h" - - export PLATFORMIO_SRC_BUILD_FLAGS="-DUSE_CUSTOM_H" - export PLATFORMIO_BUILD_CACHE_DIR="test/pio_cache" - - time pio run -s -e "$TARGET_ENVIRONMENT" - echo "travis_fold:end:build_${cfg}" -done diff --git a/travis_script.sh b/travis_script.sh index 4621fcc0..21eaf7e3 100755 --- a/travis_script.sh +++ b/travis_script.sh @@ -5,7 +5,7 @@ set -e -v cd code if [ ${TRAVIS_BUILD_STAGE_NAME} = "Test" ]; then - ./test_build.sh $BUILDER_ENV $BUILDER_EXTRA + scripts/test_build.py -e $BUILDER_ENV $BUILDER_EXTRA else ./build.sh -p fi From 01b76adff022e1b7449d46ae7a7153e495c8ece0 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 17 Oct 2019 17:45:28 +0300 Subject: [PATCH 2/7] crash,lights: ensure that settings contain positive value --- code/espurna/crash.ino | 2 +- code/espurna/light.ino | 22 +++++++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/code/espurna/crash.ino b/code/espurna/crash.ino index 9d43f15f..e047ef87 100644 --- a/code/espurna/crash.ino +++ b/code/espurna/crash.ino @@ -190,7 +190,7 @@ void crashSetup() { // Minumum of 16 and align for column formatter in crashDump() // Maximum of flash sector size minus reserved space at the beginning const uint16_t trace_max = constrain( - ((getSetting("sysTraceMax", SAVE_CRASH_STACK_TRACE_MAX).toInt() + 15) & -16), + abs((getSetting("sysTraceMax", SAVE_CRASH_STACK_TRACE_MAX).toInt() + 15) & -16), 0, (SPI_FLASH_SEC_SIZE - crashUsedSpace()) ); diff --git a/code/espurna/light.ino b/code/espurna/light.ino index e40bbc99..d1505eb1 100644 --- a/code/espurna/light.ino +++ b/code/espurna/light.ino @@ -1172,6 +1172,10 @@ void _lightAPISetup() { #if TERMINAL_SUPPORT +void _lightChannelDebug(unsigned char id) { + DEBUG_MSG_P(PSTR("Channel #%u (%s): %d\n"), id, lightDesc(id).c_str(), lightChannel(id)); +} + void _lightInitCommands() { terminalRegisterCommand(F("BRIGHTNESS"), [](Embedis* e) { @@ -1184,14 +1188,17 @@ void _lightInitCommands() { }); terminalRegisterCommand(F("CHANNEL"), [](Embedis* e) { - if (e->argc < 2) { - terminalError(F("CHANNEL []")); - return; + if (!lightChannels()) return; + + auto id = -1; + if (e->argc > 1) { + id = String(e->argv[1]).toInt(); } - const int id = String(e->argv[1]).toInt(); - if (id < 0 || id >= lightChannels()) { - terminalError(F("Channel value out of range")); + if (id < 0 || id >= static_cast(lightChannels())) { + for (unsigned char index = 0; index < lightChannels(); ++index) { + _lightChannelDebug(index); + } return; } @@ -1200,7 +1207,8 @@ void _lightInitCommands() { lightUpdate(true, true); } - DEBUG_MSG_P(PSTR("Channel #%u (%s): %d\n"), id, lightDesc(id).c_str(), lightChannel(id)); + _lightChannelDebug(id); + terminalOK(); }); From 0f2226e177d1bfeb100967131e6022a8c1383c3d Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 17 Oct 2019 17:47:04 +0300 Subject: [PATCH 3/7] sensor: cast variable as UNUSED and prevent intended assignment warnings --- code/espurna/sensor.ino | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/code/espurna/sensor.ino b/code/espurna/sensor.ino index 47a18df8..42d87221 100644 --- a/code/espurna/sensor.ino +++ b/code/espurna/sensor.ino @@ -238,6 +238,7 @@ void _sensorWebSocketOnConnected(JsonObject& root) { for (unsigned char i=0; i<_sensors.size(); i++) { BaseSensor * sensor = _sensors[i]; + UNUSED(sensor); #if EMON_ANALOG_SUPPORT if (sensor->getID() == SENSOR_EMON_ANALOG_ID) { @@ -1378,17 +1379,17 @@ void _sensorConfigure() { double value; HLW8012Sensor * sensor = (HLW8012Sensor *) _sensors[i]; - if (value = getSetting("pwrExpectedC", 0).toFloat()) { + if ((value = getSetting("pwrExpectedC", 0).toFloat())) { sensor->expectedCurrent(value); setSetting("pwrRatioC", sensor->getCurrentRatio()); } - if (value = getSetting("pwrExpectedV", 0).toInt()) { + if ((value = getSetting("pwrExpectedV", 0).toInt())) { sensor->expectedVoltage(value); setSetting("pwrRatioV", sensor->getVoltageRatio()); } - if (value = getSetting("pwrExpectedP", 0).toInt()) { + if ((value = getSetting("pwrExpectedP", 0).toInt())) { sensor->expectedPower(value); setSetting("pwrRatioP", sensor->getPowerRatio()); } From ec37c06ba50ca10f8e995f9e021c85e4f08cffbd Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 17 Oct 2019 17:47:39 +0300 Subject: [PATCH 4/7] sensor/ads1x15: fix changing sensor type when reading adc --- code/espurna/sensors/EmonADS1X15Sensor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/espurna/sensors/EmonADS1X15Sensor.h b/code/espurna/sensors/EmonADS1X15Sensor.h index e50b0460..f4ff075a 100644 --- a/code/espurna/sensors/EmonADS1X15Sensor.h +++ b/code/espurna/sensors/EmonADS1X15Sensor.h @@ -327,7 +327,7 @@ class EmonADS1X15Sensor : public EmonSensor { unsigned int readADC(unsigned char channel) { UNUSED(channel); unsigned int value = i2c_read_uint16(_address, ADS1X15_REG_POINTER_CONVERT); - if (_type = ADS1X15_CHIP_ADS1015) value >>= ADS1015_BIT_SHIFT; + if (_type == ADS1X15_CHIP_ADS1015) value >>= ADS1015_BIT_SHIFT; delayMicroseconds(500); return value; } From cb9dba4bb6f1cc369bfa278505298befa1106d19 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 17 Oct 2019 17:48:33 +0300 Subject: [PATCH 5/7] sensor/pmsx003: only delete object when it is managed by sensor --- code/espurna/sensors/PMSX003Sensor.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/code/espurna/sensors/PMSX003Sensor.h b/code/espurna/sensors/PMSX003Sensor.h index 2e0526c7..cf235982 100644 --- a/code/espurna/sensors/PMSX003Sensor.h +++ b/code/espurna/sensors/PMSX003Sensor.h @@ -158,14 +158,13 @@ class PMSX003Sensor : public BaseSensor, PMSX003 { // --------------------------------------------------------------------- // Public // --------------------------------------------------------------------- - PMSX003Sensor(): BaseSensor() { _count = pms_specs[_type].slot_count; _sensor_id = SENSOR_PMSX003_ID; } ~PMSX003Sensor() { - if (_serial) delete _serial; + removeSerial(); } void setRX(unsigned char pin_rx) { @@ -216,7 +215,7 @@ class PMSX003Sensor : public BaseSensor, PMSX003 { if (!_dirty) return; if (_soft) { - if (_serial) delete _serial; + if (_serial) removeSerial(); _serial = new SoftwareSerial(_pin_rx, _pin_tx, false, 64); static_cast(_serial)->enableIntTx(false); } @@ -358,6 +357,13 @@ class PMSX003Sensor : public BaseSensor, PMSX003 { return _slot_values[index]; } + private: + void removeSerial() { + if (_serial && _soft) { + delete static_cast(_serial); + } + } + protected: bool _soft = true; unsigned int _pin_rx; From 49bfd0819ad3fc4def3061089919e0de25c4f4da Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 17 Oct 2019 17:48:59 +0300 Subject: [PATCH 6/7] sensor/ade7953: unused variable --- code/espurna/sensors/ADE7953Sensor.h | 1 - 1 file changed, 1 deletion(-) diff --git a/code/espurna/sensors/ADE7953Sensor.h b/code/espurna/sensors/ADE7953Sensor.h index 5b920880..8420622b 100644 --- a/code/espurna/sensors/ADE7953Sensor.h +++ b/code/espurna/sensors/ADE7953Sensor.h @@ -93,7 +93,6 @@ class ADE7953Sensor : public I2CSensor { void pre() { uint32_t active_power1 = 0; uint32_t active_power2 = 0; - uint32_t current_rms = 0; uint32_t current_rms1 = 0; uint32_t current_rms2 = 0; uint32_t voltage_rms = 0; From 9025a4eb4297256f425ebfea3f0a115a44c7f1a1 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Mon, 21 Oct 2019 16:45:15 +0300 Subject: [PATCH 7/7] sensor/si7021: unused variable --- code/espurna/sensors/SI7021Sensor.h | 1 - 1 file changed, 1 deletion(-) diff --git a/code/espurna/sensors/SI7021Sensor.h b/code/espurna/sensors/SI7021Sensor.h index 3f824c28..ff2616cd 100644 --- a/code/espurna/sensors/SI7021Sensor.h +++ b/code/espurna/sensors/SI7021Sensor.h @@ -147,7 +147,6 @@ class SI7021Sensor : public I2CSensor { // When not using clock stretching (*_NOHOLD commands) delay here // is needed to wait for the measurement. // According to datasheet the max. conversion time is ~22ms - unsigned long start = millis(); nice_delay(50); // Clear the last to bits of LSB to 00.