[riot-notifications] [RIOT-OS/RIOT] drivers: Add support of TCA6507 (#12339)

Alexandre Abadie notifications at github.com
Mon Sep 30 21:53:13 CEST 2019


aabadie commented on this pull request.

Thanks for contributing to RIOT @wysman !

The driver code is already in a good shape. That great you tried to follow our coding conventions from the start.

Just a few comments:
- You are not implementing any SAUL function, so just drop the `tca6507_saul.c` file. You can add it later in a follow-up PR if you want
- Please provide a test application so the driver is built by the CI for each board supported in RIOT. The driver application should be located in `tests/driver_tca6507`
- What would also be appreciated is a bit more doxygen documentation
- If you could provide a picture (or vidéo) of the lora board controlling LEDs. I'm not sure others has this hardware and can test but at least, it shows that it works.


> @@ -0,0 +1,138 @@
+/*
+ * Copyright (C) 2019 Noel Le Moult <noel.lemoult at dfxlab.fr>
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+ /**
+  * @ingroup     drivers_actuators
+  * @ingroup     drivers_saul
+  * @defgroup    drivers_tca6507 TCA6507 I2C Led controller
+  * @brief       Internal definitions for TCA6507 I2C Led controller

This is the public API.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/12339#pullrequestreview-295206081
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190930/e51632de/attachment.htm>


More information about the notifications mailing list