Skip to content

Commit

Permalink
modules: led: Improve error handling
Browse files Browse the repository at this point in the history 
Improve error handling:
- Add missing error handling for function calls
- Cleanup log statements
- Propagate errors to the led module
- Make PWM start and stop functions static,
  remove from API. Not used externally.

Signed-off-by: Simen S. Røstad <[email protected]>
  • Loading branch information
@simensrostad
simensrostad committed Sep 30, 2024
1 parent 23c092b commit 04e46b7
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 85 deletions.
18 changes: 16 additions & 2 deletions app/src/modules/led/led.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ static void led_pattern_update_work_fn(struct k_work *work)
{
ARG_UNUSED(work);

int err;
struct led_pattern *next_pattern;
static enum led_state previous_led_state = LED_PATTERN_COUNT;
sys_snode_t *node = sys_slist_get(&pattern_transition_list);
Expand All @@ -186,12 +187,25 @@ static void led_pattern_update_work_fn(struct k_work *work)
LOG_DBG("Setting LED configuration: red: %d, green: %d, blue: %d",
next_pattern->red, next_pattern->green, next_pattern->blue);

led_pwm_set_rgb(next_pattern->red, next_pattern->green, next_pattern->blue);
err = led_pwm_set_rgb(next_pattern->red,
next_pattern->green,
next_pattern->blue);
if (err) {
LOG_ERR("Failed to set LED configuration");
SEND_FATAL_ERROR();
return;
}

} else {

LOG_DBG("Setting LED effect: %s", led_state_name(next_pattern->led_state));

led_pwm_set_effect(next_pattern->led_state);
err = led_pwm_set_effect(next_pattern->led_state);
if (err) {
LOG_ERR("Failed to set LED effect");
SEND_FATAL_ERROR();
return;
}
}

previous_led_state = next_pattern->led_state;
Expand Down
190 changes: 114 additions & 76 deletions app/src/modules/led/led_pwm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ static const struct pwm_dt_spec led2 = PWM_DT_SPEC_GET(PWM_LED2_NODE);
static K_THREAD_STACK_DEFINE(stack_area, CONFIG_APP_LED_PWM_WORKQUEUE_STACK_SIZE);
static struct k_work_q led_pwm_queue;

static struct led_effect effect_on = LED_EFFECT_LED_ON(LED_NOCOLOR());
struct led {
size_t id;
struct led_color color;
Expand All @@ -51,6 +52,8 @@ struct led {
struct k_work_sync work_sync;
};

static struct led leds;

static const struct led_effect effect[] = {
[LED_LTE_CONNECTING] = LED_EFFECT_LED_BREATHE(
LED_ON_PERIOD_NORMAL,
Expand All @@ -62,7 +65,7 @@ static const struct led_effect effect[] = {
LED_OFF_PERIOD_LONG,
LED_POLL_MODE_COLOR
),
[LED_LOCATION_SEARCHING] = LED_EFFECT_LED_BREATHE(
[LED_LOCATION_SEARCHING] = LED_EFFECT_LED_BREATHE(
LED_ON_PERIOD_NORMAL,
LED_OFF_PERIOD_NORMAL,
LED_LOCATION_SEARCHING_COLOR
Expand All @@ -84,32 +87,58 @@ static const struct led_effect effect[] = {
),
};

static struct led leds;

static void pwm_out(const struct led_color *color)
static int pwm_out(const struct led_color *color)
{
int err;

/* RED */
err = pwm_set_dt(&led0, PWM_USEC(255), PWM_USEC(color->c[0]));
if (err) {
LOG_ERR("pwm_set_dt, error:%d", err);
return;
return err;
}

/* GREEN */
err = pwm_set_dt(&led2, PWM_USEC(255), PWM_USEC(color->c[1]));
if (err) {
LOG_ERR("pwm_set_dt, error:%d", err);
return;
return err;
}

/* BLUE */
err = pwm_set_dt(&led1, PWM_USEC(255), PWM_USEC(color->c[2]));
if (err) {
LOG_ERR("pwm_set_dt, error:%d", err);
return;
return err;
}

return 0;
}

static int led_pwm_start(void)
{
int err = pm_device_action_run(led0.dev, PM_DEVICE_ACTION_RESUME);

if (err) {
LOG_ERR("PWM enable failed, pm_device_action_run: %d.", err);
return err;
}

return 0;
}

static int led_pwm_stop(void)
{
k_work_cancel_delayable_sync(&leds.work, &leds.work_sync);

int err = pm_device_action_run(led0.dev, PM_DEVICE_ACTION_SUSPEND);

if (err) {
LOG_ERR("PWM disable failed, pm_device_action_run: %d.", err);
return err;
}

return 0;
}

static void work_handler(struct k_work *work)
Expand All @@ -126,9 +155,10 @@ static void work_handler(struct k_work *work)
leds.color.c[i] += diff;
}

pwm_out(&leds.color);
__ASSERT_NO_MSG(pwm_out(&leds.color) == 0);

leds.effect_substep++;

if (leds.effect_substep == effect_step->substep_cnt) {
leds.effect_substep = 0;
leds.effect_step++;
Expand All @@ -138,125 +168,128 @@ static void work_handler(struct k_work *work)
leds.effect_step = 0;
}
} else {
__ASSERT_NO_MSG(leds.effect->steps[leds.effect_step]
.substep_cnt > 0);
__ASSERT_NO_MSG(leds.effect->steps[leds.effect_step].substep_cnt > 0);
}
}

if (leds.effect_step < leds.effect->step_cnt) {
int32_t next_delay =
leds.effect->steps[leds.effect_step].substep_time;
int32_t next_delay = leds.effect->steps[leds.effect_step].substep_time;

k_work_reschedule_for_queue(&led_pwm_queue, &leds.work, K_MSEC(next_delay));
}
}

static void led_update(struct led *led)
static int led_update(struct led *led)
{
k_work_cancel_delayable_sync(&led->work, &led->work_sync);

led->effect_step = 0;
led->effect_substep = 0;

if (led == NULL) {
LOG_ERR("Input variable is NULL");
return -EINVAL;
}

if (!led->effect) {
LOG_DBG("No effect set");
return;
return -EINVAL;
}

__ASSERT_NO_MSG(led->effect->steps);
if (led->effect->step_cnt == 0 || led->effect->steps == NULL) {
LOG_DBG("Effect steps or count is not set");
return -EINVAL;
}

if (led->effect->step_cnt > 0) {
int32_t next_delay =
led->effect->steps[led->effect_step].substep_time;
int32_t next_delay = led->effect->steps[led->effect_step].substep_time;

k_work_schedule_for_queue(&led_pwm_queue, &led->work, K_MSEC(next_delay));
} else {
LOG_DBG("LED effect with no effect");
}
k_work_schedule_for_queue(&led_pwm_queue, &led->work, K_MSEC(next_delay));

return 0;
}

void led_pwm_start(void)
int led_pwm_set_effect(enum led_state state)
{
int err;
enum pm_device_state led0_pwm_power_state;

// this action starts also led1 and led2 pwm
err = pm_device_action_run(led0.dev, PM_DEVICE_ACTION_RESUME);
if (err) {
LOG_ERR("PWM enable failed");
return;
if (!pwm_is_ready_dt(&led0)) {
LOG_ERR("PWM not ready");
return -ENODEV;
}
}

void led_pwm_stop(void)
{
int err;
k_work_cancel_delayable_sync(&leds.work, &leds.work_sync);

// this action suspends also led1 and led2 pwm
err = pm_device_action_run(led0.dev, PM_DEVICE_ACTION_SUSPEND);
err = pm_device_state_get(led0.dev, &led0_pwm_power_state);
if (err) {
LOG_ERR("PWM disable failed");
return;
LOG_ERR("Failed to assess leds pwm power state, pm_device_state_get: %d.", err);
return err;
}
}

void led_pwm_set_effect(enum led_state state)
{
int err;
enum pm_device_state led0_pwm_power_state;
LOG_DBG("Power state: %d", led0_pwm_power_state);

if (pwm_is_ready_dt(&led0)) {
err = pm_device_state_get(led0.dev, &led0_pwm_power_state);
if ((state == LED_OFF) && (led0_pwm_power_state == PM_DEVICE_STATE_ACTIVE)) {
err = led_pwm_stop();
if (err) {
LOG_ERR("Failed to assess leds pwm power state, pm_device_state_get: %d.",
err);
return;
LOG_ERR("Failed to stop leds pwm, led_pwm_stop: %d.", err);
return err;
}

if (state == LED_OFF) {
// stop leds pwm if on, and return
if (led0_pwm_power_state == PM_DEVICE_STATE_ACTIVE) {
led_pwm_stop();
}
return;
}
return 0;
}

if (led0_pwm_power_state == PM_DEVICE_STATE_SUSPENDED) {
// start leds pwm if off
led_pwm_start();
if ((led0_pwm_power_state == PM_DEVICE_STATE_SUSPENDED) ||
(led0_pwm_power_state == PM_DEVICE_STATE_SUSPENDING)) {
err = led_pwm_start();
if (err) {
LOG_ERR("Failed to start leds pwm, led_pwm_start: %d.", err);
return err;
}
}

leds.effect = &effect[state];
led_update(&leds);
}

static struct led_effect effect_on = LED_EFFECT_LED_ON(LED_NOCOLOR());
err = led_update(&leds);
if (err) {
LOG_ERR("Failed to update leds pwm, led_update: %d.", err);
return err;
}

return 0;
}

int led_pwm_set_rgb(uint8_t red, uint8_t green, uint8_t blue)
{
int err;
enum pm_device_state led0_pwm_power_state;

if (pwm_is_ready_dt(&led0)) {
err = pm_device_state_get(led0.dev, &led0_pwm_power_state);
LOG_ERR("PWM not ready");
return -ENODEV;
}

err = pm_device_state_get(led0.dev, &led0_pwm_power_state);
if (err) {
LOG_ERR("Failed to assess leds pwm power state, pm_device_state_get: %d.", err);
return err;
}

LOG_DBG("Power state: %d", led0_pwm_power_state);

if ((!red && !green && !blue) && (led0_pwm_power_state == PM_DEVICE_STATE_ACTIVE)) {
err = led_pwm_stop();
if (err) {
LOG_ERR("Failed to assess leds pwm power state, pm_device_state_get: %d.",
err);
return 0;
LOG_ERR("Failed to stop leds pwm, led_pwm_stop: %d.", err);
return err;
}

if (red == 0 && green == 0 && blue == 0) {
// stop leds pwm if on, and return
if (led0_pwm_power_state == PM_DEVICE_STATE_ACTIVE) {
led_pwm_stop();
}
return 0;
}
return 0;
}

if (led0_pwm_power_state == PM_DEVICE_STATE_SUSPENDED) {
// start leds pwm if off
led_pwm_start();
if ((led0_pwm_power_state == PM_DEVICE_STATE_SUSPENDED) ||
(led0_pwm_power_state == PM_DEVICE_STATE_SUSPENDING)) {
err = led_pwm_start();
if (err) {
LOG_ERR("Failed to start leds pwm, led_pwm_start: %d.", err);
return err;
}
}

Expand All @@ -265,7 +298,12 @@ int led_pwm_set_rgb(uint8_t red, uint8_t green, uint8_t blue)
effect_on.steps[0].color.c[2] = blue;

leds.effect = &effect_on;
led_update(&leds);

err = led_update(&leds);
if (err) {
LOG_ERR("Failed to update leds pwm, led_update: %d.", err);
return err;
}

return 0;
}
Expand Down
8 changes: 1 addition & 7 deletions app/src/modules/led/led_pwm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,8 @@
extern "C" {
#endif

/**@brief Starts the PWM and handling of LED effects. */
void led_pwm_start(void);

/**@brief Stops the LED effects. */
void led_pwm_stop(void);

/**@brief Sets LED effect based in UI LED state. */
void led_pwm_set_effect(enum led_state state);
int led_pwm_set_effect(enum led_state state);

/**@brief Sets RGB and light intensity values, in 0 - 255 ranges. */
int led_pwm_set_rgb(uint8_t red, uint8_t green, uint8_t blue);
Expand Down

0 comments on commit 04e46b7

Please sign in to comment.