New feature: digital output for controlling lighting with HAL #2675

shellixyz · 2018-01-22T14:50:08Z

This PR adds the possibility to control lighting with a digital output.

To use this functionality you need to build a custom firmware after defining the pin to use as output in the USE_LIGHTS macro (example on PIKOBLX to use the PWM5 output: #define LIGHTS_PIN PA1)
No overhead if the USE_LIGHTS macro is not defined
Lights flashing can be enabled on failsafe with the failsafe_lights setting and flashing on time and period can be configured with the failsafe_lights_flash_period and failsafe_lights_flash_on_time settings (unit: milliseconds).
The output can be switched on with the new LIGHTS flight mode.

Example application (don't forget the current limiting resistor(s) if you are using LEDs which don't have built-in current limiting):

DzikuVx · 2018-01-22T15:33:47Z

it's connected with #296

DzikuVx · 2018-01-22T15:36:10Z

Would you belive I wanted to do it more than a year ago?
I think it would be nice to add additioanl level of abstraction from the start that will allow to use external driver over I2C for example like PCF8574 so it can be configurable if to use FC PWM outputs or extarnal device

DzikuVx · 2018-01-22T15:31:45Z

src/main/fc/rc_modes.h

@@ -56,6 +56,7 @@ typedef enum {
    BOXCAMERA1      = 29,
    BOXCAMERA2      = 30,
    BOXCAMERA3      = 31,
+    BOXLIGHTS       = 32,


There already is BOXLLIGHTS = 13,

I added a mode in case somebody wants to use RGB and standard lighting at the same time but I can modify the code to use the LLIGHTS mode without a problem.

LLIGHTS are not used, it's not connected with RGB LEDs at all. You mistake it with BOXLEDLOW

BOXLLIGHTS stands for Landing Lights

Done. Now using old unused BOXLLIGHTS IDs.

DzikuVx · 2018-01-22T15:32:54Z

src/main/fc/fc_msp_box.c

@@ -70,6 +70,7 @@ static const box_t boxes[CHECKBOX_ITEM_COUNT + 1] = {
    { BOXCAMERA1, "CAMERA CONTROL 1;", 39 },
    { BOXCAMERA2, "CAMERA CONTROL 2;", 40 },
    { BOXCAMERA3, "CAMERA CONTROL 3;", 41 },
+    { BOXLIGHTS, "LIGHTS;", 42 },


{ BOXLLIGHTS, "LLIGHTS;", 16 },

shellixyz · 2018-01-22T15:48:20Z

I believe it :)
About the abstraction I'm all for it but I'm not sure how to proceed.

DzikuVx · 2018-01-22T16:10:46Z

I suppose we can skip abstraction for now.
By abstraction I ment additional 2 files. light.c write to abstraction and then abstraction uses either FC pwms or any new driver. So when you add new hardware, you do not change lights.c but only a "driver". We have this for rangefinder for example

Linjieqiang · 2018-01-23T04:21:03Z

Why not use WS2812 LEDStrip as the lighting for fixedwing?

DzikuVx · 2018-01-23T09:24:54Z

@Linjieqiang because with digital output you can connect and drive everything. Like 10W LED or IR emitter or anything else

martinbudden · 2018-01-23T09:41:35Z

src/main/io/lights.c

+    return(true);
+  } else
+    return(false);
+}


Please use standard 4 space indentation.

Done. I need to change my auto-indentation settings for iNav it is not the same I use for other projects.

martinbudden · 2018-01-23T09:43:47Z

src/main/fc/fc_tasks.c

@@ -314,6 +315,9 @@ void fcTasksInit(void)
    setTaskEnabled(TASK_SERIAL, true);
 #ifdef BEEPER
    setTaskEnabled(TASK_BEEPER, true);
+#endif
+#ifdef LIGHTS


This should be #ifdef USE_LIGHTS - the USE_ prefix is used for this type of build flag.

OK. I didn't because it is not only a flag it is used to define the pin to use. Maybe best to use two flags: USE_LIGHTS and LIGHTS_PIN ?

Done. Macros to use are now USE_LIGHTS, USE_FAILSAFE_LIGHTS and LIGHTS_PIN.

martinbudden · 2018-01-23T09:45:51Z

src/main/fc/fc_tasks.c

+        .taskName = "LIGHTS",
+        .taskFunc = lightsUpdate,
+        .desiredPeriod = TASK_PERIOD_HZ(100),     // 100 Hz
+        .staticPriority = TASK_PRIORITY_MEDIUM,


priority should be low.

OK. I used the same priority as the buzzer/beeper because it is also used to flash lights on failsafe. Maybe the buzzer/beeper priority can be set to LOW too ?

Done. Task priority is now LOW.

martinbudden · 2018-01-23T09:47:00Z

Good stuff. However I'm not sure if this should be called LIGHTS, since, as @DzikuVx , points out, you can connect anything to the output.

kabturek · 2018-01-23T10:20:31Z

Arduplane calls it relay switches

shellixyz · 2018-01-23T13:47:36Z

@martinbudden I called it LIGHTS because it is used to flash lights on failsafe. Not sure it would be relevant for something else in this context.

shellixyz · 2018-01-23T18:45:29Z

~~@DzikuVx I had a go at the HAL please check out #2681.~~

Update: The HAL has been merged into this PR

…SERT

DzikuVx

I'm fine with this

digitalentity

Noticed a few things to think about.

digitalentity · 2018-01-25T07:34:44Z

src/main/fc/fc_tasks.c

+    [TASK_LIGHTS] = {
+        .taskName = "LIGHTS",
+        .taskFunc = lightsUpdate,
+        .desiredPeriod = TASK_PERIOD_HZ(100),     // 100 Hz


Isn't 100Hz an overkill? Do we want to update light status 100 times per second? I think 10Hz whould be enough.

It is needed for the failsafe lights flashing code. As the default I set pulses the lights 100ms every second. It is what I use to keep battery usage low in case the model is lost and a long time is needed to find it. If we set it to 10Hz it won't be possible as it will make the lights either not light up or light up 100ms or 200ms each cycle depending on the time variability between task runs. The task priority is set to LOW it shouldn't be a problem.

digitalentity · 2018-01-25T07:37:28Z

src/main/drivers/lights_hal.c

@@ -0,0 +1,30 @@
+#include "drivers/lights_hal.h"


I don't like the name lights_hal. We use _hal suffix in drivers that depend on ST's HAL library. This is clearly not the case. Maybe use lights_io name here?

As you like. I thought it would be appropriate as it is already used for WS2811 lights: light_ws2811strip_hal.c.

Exactly my case - light_ws2811strip_hal.c is driver designed to work with ST's HAL library. That's why we have light_ws2811strip_stdperiph.c next to is (using ST's StdPeriph library). We don't depend on ST's CPU-support libraries, so let's rename.

digitalentity · 2018-01-25T07:38:28Z

src/main/io/lights.c

+void lightsUpdate(timeUs_t currentTimeUs)
+{
+    UNUSED(currentTimeUs);
+#ifdef USE_FAILSAFE_LIGHTS


Why we have USE_FAILSAFE_LIGHTS? Why not make it a config option rather than compile-time option?

I thought it would not be a setting changed often but you are right it would be better as a run-time setting. I will change that.

Done. This PR has been tested and should be ready to merge.

…ghts_io.{c,h}

…d failsafe_lights_flash_on_time minimums

krzysztofmatula · 2018-02-07T12:40:34Z

Heh, I was proposing something similar back in 2015 :)
cleanflight/cleanflight#1603

krzysztofmatula · 2018-02-07T12:49:34Z

Now I use module like this one with the led desoldered and replaces with N_MOSFET to drive high power outputs: https://pl.aliexpress.com/item/50pcs-DC5V-WS2811-IC-RGB-led-Pixel-node-Module-Light-No-Wire-Addressable-12mm-for-led/32794166762.html?spm=a2g17.search0104.3.1.302d7981WVnhBj&ws_ab_test=searchweb0_0,searchweb201602_2_10065_10344_10068_10342_10343_10340_10341_10084_10083_10618_10304_10307_10301_5722315_10313_10059_10534_100031_10103_441_10624_442_10623_10622_10621_10620_10142,searchweb201603_25,ppcSwitch_5&algo_expid=94ee305e-c3f3-4ba5-b45c-c686a38e0bbe-0&algo_pvid=94ee305e-c3f3-4ba5-b45c-c686a38e0bbe&priceBeautifyAB=0

New feature: digital output for controlling lighting

cbada75

DzikuVx requested changes Jan 22, 2018

View reviewed changes

martinbudden reviewed Jan 23, 2018

View reviewed changes

shellixyz added 5 commits January 23, 2018 14:55

digital lights control: fix indentation

3e368d9

digital lights control: use old unused BOXLLIGHTS id

bd7fc77

digital lights control: add USE_ prefix

c96c4fc

digital lights control: change task priority to LOW

7174277

lighting digital output HAL

b78d8f7

shellixyz mentioned this pull request Jan 23, 2018

Lighting digital output HAL #2681

Closed

shellixyz added 3 commits January 24, 2018 19:25

digital lights control: remove PCA9685 placeholder

fec1854

digital lights control: adjust rc_modes CHECKBOX_ITEM_COUNT STATIC_AS…

87f1d95

…SERT

Merge branch 'lighting_digital_output_hal' into lighting_digital_output

d4c34b3

shellixyz changed the title ~~New feature: digital output for controlling lighting~~ New feature: digital output for controlling lighting with HAL Jan 24, 2018

DzikuVx approved these changes Jan 24, 2018

View reviewed changes

digitalentity requested changes Jan 25, 2018

View reviewed changes

shellixyz added 3 commits January 25, 2018 17:42

digital lights control: rename drivers/lights_hal.{c,h} to drivers/li…

e0d5543

…ghts_io.{c,h}

digital lights control: fix bug introduced with HAL impl.

cc51913

digital lights control: failsafe flashing run-time config

1a2a18f

digital lights control: more sensible failsafe_lights_flash_period an…

742a789

…d failsafe_lights_flash_on_time minimums

digitalentity approved these changes Jan 26, 2018

View reviewed changes

digitalentity added this to the 1.9 milestone Jan 26, 2018

digitalentity merged commit 873946d into iNavFlight:development Jan 26, 2018

shellixyz deleted the lighting_digital_output branch February 3, 2018 17:57

New feature: digital output for controlling lighting with HAL #2675

New feature: digital output for controlling lighting with HAL #2675

Conversation

shellixyz commented Jan 22, 2018 • edited Loading

DzikuVx commented Jan 22, 2018

DzikuVx commented Jan 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DzikuVx Jan 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shellixyz commented Jan 22, 2018

DzikuVx commented Jan 22, 2018

Linjieqiang commented Jan 23, 2018

DzikuVx commented Jan 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinbudden commented Jan 23, 2018

kabturek commented Jan 23, 2018

shellixyz commented Jan 23, 2018

shellixyz commented Jan 23, 2018 • edited Loading

DzikuVx left a comment

Choose a reason for hiding this comment

digitalentity left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shellixyz Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

krzysztofmatula commented Feb 7, 2018

krzysztofmatula commented Feb 7, 2018

shellixyz commented Jan 22, 2018 •

edited

Loading

DzikuVx Jan 22, 2018 •

edited

Loading

shellixyz commented Jan 23, 2018 •

edited

Loading

shellixyz Jan 25, 2018 •

edited

Loading