feature: shift register #458

JaimeAlbq · 2022-10-09T21:19:46Z

shift_reg: add shift_reg component

The feature is a driver to interface the microcontroller with shift
registers just using 3 pins: Clock, Data and Latch.

For further information, please, take a look at the component
README.md.

Feature requested by issue #457.

trombik · 2022-10-09T22:48:50Z

i made the PR draft. when the PR passes the tests, please remove Draft. when you are ready for review, ask a review. if you need help to pass tests, ask here.

please consolidate commits, especially when you reverted commits, by git rebase -i. see A Guide to Git Interactive Rebase, with Practical Examples. in general, too small commits is better than too big commits.

(so far, looks good)

trombik · 2022-10-11T22:25:55Z

here is my first review. the biggest one is the order of arguments.

all in all, a good PR.

JaimeAlbq · 2022-10-11T22:35:00Z

here is my first review. the biggest one is the order of arguments.

all in all, a good PR.

No problem. If you need anything from my side or a hand, I'll be there.

trombik

please see my comments. for starters, change the order of arguments. thanks

components/shift_reg/shift_reg.c

components/shift_reg/shift_reg.h

examples/shift_reg/default/main/main.c

The feature is a driver to interface the microcontroller with shift registers just using 3 pins: Clock, Data and Latch. For further information, please, take a look at the component [README.md](components/shift_reg/README.md). Feature requested by issue UncleRus#457.

JaimeAlbq · 2022-10-20T11:20:14Z

Hello @trombik,

Could you please perform the review one more time. I did the required changes.

trombik · 2022-10-20T11:34:05Z

@JaimeAlbq will do after the weekend (I work on weekends)

UncleRus · 2022-10-24T13:55:28Z

components/shift_reg/shift_reg.c

+        // MSB Mode
+        for (int8_t i = 7; i >= 0; i--)
+        {
+            if ((data >> i) & 1)


This condition can be replaced by a single expression:

gpio_set_level(dev->pin.data, (data >> i) & 1);

components/shift_reg/shift_reg.c

UncleRus · 2022-10-24T13:56:20Z

components/shift_reg/shift_reg.c

+
+esp_err_t shift_reg_latch(shift_reg_config_t *dev)
+{
+    gpio_set_level(dev->pin.latch, true);


Here we need to check if dev is NULL

UncleRus · 2022-10-24T13:57:01Z

components/shift_reg/shift_reg.h

+#include <driver/gpio.h>
+#include <ets_sys.h>
+
+typedef enum {


All structs and enums must be commented in doxy-style, like this:

/** * This structure is intended to store the device handle and its state. */ typedef struct { int foo; //< Special field } some_device_t; ... /** * Device operational modes */ typedef enum { MODE_FOO = 0; //< Mode Foo MODE_BAR; //< Mode Bar } some_mode_t;

UncleRus · 2022-10-24T13:59:08Z

examples/shift_reg/default/sdkconfig.defaults

@@ -0,0 +1 @@
+# add required non-default option for the example if any


Please delete this file if the example does not require special ESP-IDF settings.

trombik · 2022-10-26T07:40:13Z

examples/shift_reg/default/main/main.c

+			ESP_ERROR_CHECK(shift_reg_send(&shifter, value, 2));
+			ESP_ERROR_CHECK(shift_reg_latch(&shifter));
+			ESP_LOGI(__func__, "reg_value: 0x%02X 0x%02X", shifter.reg_value[0], shifter.reg_value[1]);
+			vTaskDelay(2 / portTICK_PERIOD_MS);


use pdMS_TO_TICKS instead of portTICK_PERIOD_MS which is deprecated.

trombik · 2022-10-26T07:49:54Z

components/shift_reg/README.md

+To initialize the shift register it is going to need to call the `esp_err_t shift_reg_init(shift_reg_config_t *dev)`, by passing the created shift register configurations.
+
+### De-initialization
+Since the `uint8_t *reg_value` is created accordingly of the number of registers, the vector allocate the necessary size in the heap memory, so `esp_err_t shift_reg_deinit(shift_reg_config_t *dev)` can be used to free this memory.


this should go to the header file so that users can see this in the official documentation. see other documentation pages under docs/source/groups.

trombik · 2022-11-14T13:38:15Z

@JaimeAlbq please read the review comments, and modify the code, or justify. after that, I'll test the code on real hardware.

- `examples/shift_reg/default/sdkconfig.defaults`: removed - `README.md`: moved and renamed to `docs/source/groups/shift_reg.rst` - `examples/shift_reg/default/main/main.c`: updated the `portTICK_PERIOD_MS` to `pdMS_TO_TICKS` - `components/shift_reg/shift_reg.h`: added comment in doxygen-style for structures and enums - `components/shift_reg/shift_reg.c`: - `shift_reg_latch()`: added argument checker for `NULL` pointer - `shift_reg_send8bits()`: improved the GPIO set

`.eil.yml`: updated

JaimeAlbq · 2024-02-07T14:25:16Z

@UncleRus or @trombik, could one of you support me to pass the CI?

mmarkwort · 2024-05-10T10:17:36Z

Call devtools.py from devtools directory with arguments check. If check is okay then call with argument render.

There are some doc files are missing and the tool should help you to generate them.

mmarkwort · 2024-05-10T10:34:26Z

I did a small check on your branch. You should update the branch to the current master version. Then first you have to add your name to the devtools/persons.yml. Then you have to remove 'name:' from depends section in your .eil.yml

After you have made the changes run devtooly.py check and devtooly.py render again.

UncleRus added area:ci enhancement New feature or request labels Oct 9, 2022

trombik marked this pull request as draft October 9, 2022 22:38

trombik changed the title ~~Feature shift register~~ feature: shift register Oct 9, 2022

JaimeAlbq force-pushed the feature-shift-reg branch 2 times, most recently from 01ae8f9 to 39471fb Compare October 11, 2022 15:57

UncleRus added the area:docs label Oct 11, 2022

JaimeAlbq marked this pull request as ready for review October 11, 2022 16:35

trombik requested changes Oct 12, 2022

View reviewed changes

JaimeAlbq force-pushed the feature-shift-reg branch from 39471fb to 5134fda Compare October 19, 2022 11:56

JaimeAlbq requested a review from trombik October 19, 2022 12:41

trombik self-assigned this Oct 23, 2022

UncleRus reviewed Oct 24, 2022

View reviewed changes

components/shift_reg/shift_reg.c Show resolved Hide resolved

UncleRus reviewed Oct 24, 2022

View reviewed changes

trombik reviewed Oct 26, 2022

View reviewed changes

UncleRus marked this pull request as draft November 7, 2022 03:44

UncleRus removed area:ci labels Aug 4, 2023

JaimeAlbq and others added 7 commits February 7, 2024 10:57

shift_reg: conflict fix part 1

eaaf6bb

shift_reg: conflict fix part 2

b639329

shift_reg: conflict fix part 3

90b4edb

Merge branch 'UncleRus:master' into feature-shift-reg

ff3d19d

shift_reg: conflict fix part 3

b2a3ec9

`.eil.yml`: updated

shift_reg: conflict fix part 5

177b64a

`.eil.yml`: updated

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: shift register #458

feature: shift register #458

JaimeAlbq commented Oct 9, 2022

trombik commented Oct 9, 2022

trombik commented Oct 11, 2022

JaimeAlbq commented Oct 11, 2022

trombik left a comment

JaimeAlbq commented Oct 20, 2022

trombik commented Oct 20, 2022

UncleRus Oct 24, 2022

UncleRus Oct 24, 2022

UncleRus Oct 24, 2022

UncleRus Oct 24, 2022

trombik Oct 26, 2022

trombik Oct 26, 2022

trombik commented Nov 14, 2022

JaimeAlbq commented Feb 7, 2024

mmarkwort commented May 10, 2024 •

edited

Loading

mmarkwort commented May 10, 2024

		@@ -0,0 +1 @@
		# add required non-default option for the example if any

feature: shift register #458

Are you sure you want to change the base?

feature: shift register #458

Conversation

JaimeAlbq commented Oct 9, 2022

trombik commented Oct 9, 2022

trombik commented Oct 11, 2022

JaimeAlbq commented Oct 11, 2022

trombik left a comment

Choose a reason for hiding this comment

JaimeAlbq commented Oct 20, 2022

trombik commented Oct 20, 2022

UncleRus Oct 24, 2022

Choose a reason for hiding this comment

UncleRus Oct 24, 2022

Choose a reason for hiding this comment

UncleRus Oct 24, 2022

Choose a reason for hiding this comment

UncleRus Oct 24, 2022

Choose a reason for hiding this comment

trombik Oct 26, 2022

Choose a reason for hiding this comment

trombik Oct 26, 2022

Choose a reason for hiding this comment

trombik commented Nov 14, 2022

JaimeAlbq commented Feb 7, 2024

mmarkwort commented May 10, 2024 • edited Loading

mmarkwort commented May 10, 2024

mmarkwort commented May 10, 2024 •

edited

Loading