Skip to content

Commit

Permalink
Enabled error on Warning in strict mode
Browse files Browse the repository at this point in the history
Fixed [-Wparentheses] parentheses around ‘-’ inside ‘<<’ BATTERY_VOLTAGE_SHUTDOWN_10_BIT

Fixed [-Wdiscarded-qualifiers]   return discards ‘volatile’ qualifier from pointer target type &m_configuration_variables;

Fixed [-Wsign-compare]  comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’

Fixed [-Wunused-but-set-variable] enable walk assist debounce code path by default

Fixed [-Wunused-label]

Fixed warning C4244: '=': conversion from 'int16_t' to 'uint8_t', possible loss of data
  • Loading branch information
dzid26 committed Jul 16, 2024
1 parent 5452e26 commit df8b31d
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 27 deletions.
7 changes: 5 additions & 2 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ jobs:


Tests:
runs-on: ubuntu-latest
runs-on: ubuntu-20.04
permissions:
contents: read
pull-requests: write
Expand All @@ -112,7 +112,10 @@ jobs:
rm tests/sim/_tsdz2.cdef # make sure cdef is generated from the source to check testing framework
echo "REPORT_FILE=${REPORT_OUTPUT}" >> "$GITHUB_ENV"
pytest --coverage --md-report --md-report-flavor gfm --md-report-output "$REPORT_OUTPUT"
gcc -Wall -Wextra -Q --help=warning # print warnings that shouldbe enabled by pytest --strict
pytest --strict --coverage --md-report --md-report-flavor gfm --md-report-output "$REPORT_OUTPUT"
- name: Output reports to the job summary
if: always()
shell: bash
Expand Down
6 changes: 3 additions & 3 deletions src/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
#define WHEEL_MAX_SPEED 25
#define ENABLE_LIGHTS 1
#define ENABLE_WALK_ASSIST 1
#define ENABLE_BRAKE_SENSOR 0
#define ENABLE_BRAKE_SENSOR 1
#define ENABLE_THROTTLE 0
#define ENABLE_TEMPERATURE_LIMIT 0
#define ENABLE_STREET_MODE_ON_STARTUP 1
Expand Down Expand Up @@ -111,7 +111,7 @@
#define WALK_ASSIST_LEVEL_3 40
#define WALK_ASSIST_LEVEL_4 45
#define WALK_ASSIST_THRESHOLD_SPEED_X10 60
#define WALK_ASSIST_DEBOUNCE_ENABLED 0
#define WALK_ASSIST_DEBOUNCE_ENABLED 1
#define WALK_ASSIST_DEBOUNCE_TIME 60
#define CRUISE_TARGET_SPEED_LEVEL_1 15
#define CRUISE_TARGET_SPEED_LEVEL_2 18
Expand All @@ -126,7 +126,7 @@
#define ASSIST_THROTTLE_MAX_VALUE 255
#define STREET_MODE_WALK_ENABLED 1
#define DATA_DISPLAY_ON_STARTUP 1
#define FIELD_WEAKENING_ENABLED 0
#define FIELD_WEAKENING_ENABLED 1
#define PEDAL_TORQUE_ADC_OFFSET_ADJ 20
#define PEDAL_TORQUE_ADC_RANGE_ADJ 20
#define PEDAL_TORQUE_ADC_ANGLE_ADJ 36
Expand Down
21 changes: 10 additions & 11 deletions src/ebike_app.c
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ static void apply_power_assist(void)
ui8_adc_battery_current_target = ui8_adc_battery_current_max;
}
else {
ui8_adc_battery_current_target = ui16_adc_battery_current_target;
ui8_adc_battery_current_target = (uint8_t)ui16_adc_battery_current_target;
}

#if STARTUP_ASSIST_ENABLED
Expand Down Expand Up @@ -815,7 +815,7 @@ static void apply_torque_assist(void)
ui8_adc_battery_current_target = ui8_adc_battery_current_max;
}
else {
ui8_adc_battery_current_target = ui16_adc_battery_current_target_torque_assist;
ui8_adc_battery_current_target = (uint8_t)ui16_adc_battery_current_target_torque_assist;
}

#if STARTUP_ASSIST_ENABLED
Expand Down Expand Up @@ -865,7 +865,7 @@ static void apply_cadence_assist(void)
ui8_adc_battery_current_target = ui8_adc_battery_current_max;
}
else {
ui8_adc_battery_current_target = ui16_adc_battery_current_target_cadence_assist;
ui8_adc_battery_current_target = (uint8_t)ui16_adc_battery_current_target_cadence_assist;
}

// set duty cycle target
Expand Down Expand Up @@ -926,7 +926,7 @@ static void apply_emtb_assist(void)
ui8_adc_battery_current_target = ui8_adc_battery_current_max;
}
else {
ui8_adc_battery_current_target = ui16_adc_battery_current_target_eMTB_assist;
ui8_adc_battery_current_target = (uint8_t)ui16_adc_battery_current_target_eMTB_assist;
}

#if STARTUP_ASSIST_ENABLED
Expand Down Expand Up @@ -1014,7 +1014,7 @@ static void apply_hybrid_assist(void)
ui8_adc_battery_current_target = ui8_adc_battery_current_max;
}
else {
ui8_adc_battery_current_target = ui16_adc_battery_current_target;
ui8_adc_battery_current_target = (uint8_t)ui16_adc_battery_current_target;
}

#if STARTUP_ASSIST_ENABLED
Expand Down Expand Up @@ -1435,7 +1435,7 @@ static void apply_temperature_limiting(void)
}
else {
// adjust target current if motor over temperature limit
ui8_adc_battery_current_target = map_ui16((uint16_t) ui16_motor_temperature_filtered_x10,
ui8_adc_battery_current_target = (uint8_t)map_ui16((uint16_t) ui16_motor_temperature_filtered_x10,
(uint16_t) ((uint8_t)ui8_motor_temperature_min_value_to_limit_array[TEMPERATURE_SENSOR_TYPE] * (uint8_t)10U),
(uint16_t) ((uint8_t)ui8_motor_temperature_max_value_to_limit_array[TEMPERATURE_SENSOR_TYPE] * (uint8_t)10U),
ui8_adc_battery_current_target,
Expand All @@ -1448,7 +1448,7 @@ static void apply_speed_limit(void)
{
if (m_configuration_variables.ui8_wheel_speed_max) {
// set battery current target
ui8_adc_battery_current_target = map_ui16((uint16_t) ui16_wheel_speed_x10,
ui8_adc_battery_current_target = (uint8_t)map_ui16((uint16_t) ui16_wheel_speed_x10,
(uint16_t) (((uint8_t)(m_configuration_variables.ui8_wheel_speed_max) * (uint8_t)10U) - (uint8_t)20U),
(uint16_t) (((uint8_t)(m_configuration_variables.ui8_wheel_speed_max) * (uint8_t)10U) + (uint8_t)20U),
ui8_adc_battery_current_target,
Expand Down Expand Up @@ -1647,9 +1647,8 @@ static void get_pedal_torque(void)
}


struct_configuration_variables* get_configuration_variables(void)
{
return &m_configuration_variables;
struct_configuration_variables* get_configuration_variables(void) {
return (struct_configuration_variables*) &m_configuration_variables;
}


Expand Down Expand Up @@ -2926,7 +2925,7 @@ static void uart_send_package(void)

// reserved for VLCD5, torque sensor value TE and TE1
#if ENABLE_VLCD5
ui8_tx_buffer[3] = ui16_adc_pedal_torque_offset_init;
ui8_tx_buffer[3] = (uint8_t)ui16_adc_pedal_torque_offset_init;
if (ui16_adc_pedal_torque > ui16_adc_pedal_torque_offset_init) {
ui8_tx_buffer[4] = ui16_adc_pedal_torque - ui16_adc_pedal_torque_offset_init;
}
Expand Down
4 changes: 2 additions & 2 deletions src/main.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ HALL_COUNTER_OFFSET_UP: 29 -> 44
#define HALL_COUNTER_OFFSET_UP (HALL_COUNTER_OFFSET_DOWN + 21)
#define FW_HALL_COUNTER_OFFSET_MAX 5 // 5*4=20us max time offset

#define MOTOR_ROTOR_INTERPOLATION_MIN_ERPS 10
#define MOTOR_ROTOR_INTERPOLATION_MIN_ERPS 10U

// adc torque offset gap value for error
#define ADC_TORQUE_SENSOR_OFFSET_THRESHOLD 30
Expand Down Expand Up @@ -372,7 +372,7 @@ HALL_COUNTER_OFFSET_UP: 29 -> 44
// battery voltage to be subtracted from the cut-off 8bit
#define DIFFERENCE_CUT_OFF_SHUTDOWN_8_BIT 26
// battery voltage for saving battery capacity at shutdown
#define BATTERY_VOLTAGE_SHUTDOWN_8_BIT (uint8_t) ((uint16_t)(BATTERY_LOW_VOLTAGE_CUT_OFF * 250 / BATTERY_VOLTAGE_PER_10_BIT_ADC_STEP_X1000)) - ((uint16_t) DIFFERENCE_CUT_OFF_SHUTDOWN_8_BIT)
#define BATTERY_VOLTAGE_SHUTDOWN_8_BIT (uint8_t) ((uint16_t)((BATTERY_LOW_VOLTAGE_CUT_OFF * 250 / BATTERY_VOLTAGE_PER_10_BIT_ADC_STEP_X1000)) - ((uint16_t) DIFFERENCE_CUT_OFF_SHUTDOWN_8_BIT))
#define BATTERY_VOLTAGE_SHUTDOWN_10_BIT (uint16_t) (BATTERY_VOLTAGE_SHUTDOWN_8_BIT << 2)
// max battery power div25
#define TARGET_MAX_BATTERY_POWER_DIV25 (uint8_t)(TARGET_MAX_BATTERY_POWER / 25)
Expand Down
10 changes: 5 additions & 5 deletions src/motor.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ static uint16_t ui16_cadence_sensor_ticks_counter_min = CADENCE_SENSOR_CALC_COUN
static uint8_t ui8_pas_state_old = 4;
static uint16_t ui16_cadence_calc_counter, ui16_cadence_stop_counter;
static uint8_t ui8_cadence_calc_ref_state = NO_PAS_REF;
const static uint8_t ui8_pas_old_valid_state[4] = { 0x01, 0x03, 0x00, 0x02 };
static const uint8_t ui8_pas_old_valid_state[4] = { 0x01, 0x03, 0x00, 0x02 };

// wheel speed sensor
volatile uint16_t ui16_wheel_speed_sensor_ticks = 0;
Expand Down Expand Up @@ -355,7 +355,7 @@ void TIM1_CAP_COM_IRQHandler(void) __interrupt(TIM1_CAP_COM_IRQHANDLER)
else {
// Verify if rotor stopped (< 10 ERPS)
// ui16_a - ui16_b = Hall counter ticks from the last Hall sensor transition;
if ((ui16_a - ui16_b) > (HALL_COUNTER_FREQ/MOTOR_ROTOR_INTERPOLATION_MIN_ERPS/6)) {
if ((uint16_t)(ui16_a - ui16_b) > (HALL_COUNTER_FREQ/MOTOR_ROTOR_INTERPOLATION_MIN_ERPS/6U)) {
ui8_motor_commutation_type = BLOCK_COMMUTATION;
ui8_g_foc_angle = 0;
ui8_hall_360_ref_valid = 0;
Expand Down Expand Up @@ -867,7 +867,7 @@ void TIM1_CAP_COM_IRQHandler(void) __interrupt(TIM1_CAP_COM_IRQHANDLER)
else { ui16_wheel_speed_sensor_ticks_counter_min = WHEEL_SPEED_SENSOR_TICKS_COUNTER_MIN >> 3; }

if (!ui8_wheel_speed_sensor_ticks_counter_started ||
(ui16_wheel_speed_sensor_ticks_counter > ui16_wheel_speed_sensor_ticks_counter_min)) {
(ui16_wheel_speed_sensor_ticks_counter > ui16_wheel_speed_sensor_ticks_counter_min)) {
// check if wheel speed sensor pin state has changed
if (ui8_temp != ui8_wheel_speed_sensor_pin_state_old) {
// update old wheel speed sensor pin state
Expand Down Expand Up @@ -897,7 +897,7 @@ void TIM1_CAP_COM_IRQHandler(void) __interrupt(TIM1_CAP_COM_IRQHANDLER)
}

// increment and also limit the ticks counter
if (ui8_wheel_speed_sensor_ticks_counter_started)
if (ui8_wheel_speed_sensor_ticks_counter_started) {
if (ui16_wheel_speed_sensor_ticks_counter < WHEEL_SPEED_SENSOR_TICKS_COUNTER_MIN) {
++ui16_wheel_speed_sensor_ticks_counter;
}
Expand All @@ -907,6 +907,7 @@ void TIM1_CAP_COM_IRQHandler(void) __interrupt(TIM1_CAP_COM_IRQHANDLER)
ui16_wheel_speed_sensor_ticks_counter = 0;
ui8_wheel_speed_sensor_ticks_counter_started = 0;
}
}


/****************************************************************************/
Expand Down Expand Up @@ -1018,7 +1019,6 @@ void TIM1_CAP_COM_IRQHandler(void) __interrupt(TIM1_CAP_COM_IRQHANDLER)
}

/****************************************************************************/
irq_end:
// clears the TIM1 interrupt TIM1_IT_UPDATE pending bit
TIM1->SR1 = (uint8_t) (~(uint8_t) TIM1_IT_CC4);
}
Expand Down
5 changes: 4 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ def pytest_sessionstart(session):
Called after the Session object has been created and
before performing collection and entering the run test loop.
"""
lib, _ = load_code('_tsdz2', coverage=session.config.option.coverage, force_recompile=session.config.option.force)
lib, _ = load_code('_tsdz2',
coverage=session.config.option.coverage,
force_recompile=session.config.option.force,
strict=session.config.option.strict)
if session.config.option.coverage:
lib.__gcov_reset()

Expand Down
8 changes: 5 additions & 3 deletions tests/load_c_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,12 @@ def generate_cdef(module_name, src_file):
fp.write(cdef)
return cdef

def load_code(module_name, coverage=False, force_recompile=False):
def load_code(module_name, coverage=False, force_recompile=False, strict=False):
# Load previous combined hash
hash_file_path = os.path.join(LIB_DIR, f"{module_name}.sha")
# Recalculate hash if code or arguments have changed
with Checksum(hash_file_path, source_dirs, module_name+"".join(define_macros)) as skip:
if not skip or force_recompile or coverage: # also recompile if coverage is enabled because backround test runners are compiling without gcov
if not skip or force_recompile or coverage or strict: # also recompile if coverage is enabled because backround test runners are compiling without gcov
print("Collecting source code..")
source_content_list: List[str] = []
source_files = [os.path.abspath(os.path.join(dir, file)) for dir in source_dirs for file in os.listdir(dir) if file.endswith('.c')]
Expand Down Expand Up @@ -188,11 +188,13 @@ def load_code(module_name, coverage=False, force_recompile=False):
combined_source = "// GCOVR_EXCL_STOP\n" + combined_source + "\n// GCOVR_EXCL_START"
extra_compile_args += ["--coverage"]
extra_link_args += ["--coverage"]

if strict:
extra_compile_args += ["-Werror"]
# Create a CFFI instance
ffibuilder = cffi.FFI()
print("Processing cdefs...")
ffibuilder.cdef(cdef)
print("Compiler args:", extra_compile_args, *include_dirs)
ffibuilder.set_source(module_name, combined_source,
include_dirs=[os.path.abspath(d) for d in include_dirs],
define_macros=[(macro, None) for macro in define_macros],
Expand Down

0 comments on commit df8b31d

Please sign in to comment.