Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pkg/arduino_lis3dh: Arduino library for LIS3DH #12518

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

pkg/arduino_lis3dh: Arduino library for LIS3DH #12518

wants to merge 6 commits into from
+343 −0

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Oct 21, 2019 •
edited
Loading

Contribution description

This PR was initially provided for testing PR #12180, #12118 and #10592 along with a complex library and a known sensor. Now where these PRs are merged, the PR is for demonstrating and testing the Adafruit Unified Sensor Driver package in PR #17460.

It includes PR #17460 (commits ac18140 and fa7d4d9).

Testing procedure

Compilation should succeed in CI. The driver package can be tested with any board that support arduino feature.

BOARD=esp32-wroom-32 make -C tests/pkg_arduino_lis3dh flash term.

Issues/PRs references

Used for PR #12180, #12118 and #10592
Depends on PR #17460

This was referenced Oct 21, 2019
Merged
Merged
@smlng smlng added Area: arduino API Area: Arduino wrapper API Area: pkg Area: External package ports Area: tests Area: tests and testing framework labels Oct 21, 2019
@gschorcht gschorcht mentioned this pull request Oct 22, 2019
Merged
@gschorcht gschorcht added the State: demonstrator State: This PR (loosely) demonstrates a concept and is not necessarily meant to be merged. label Oct 27, 2019
@benpicco
Copy link
Contributor

I was a bit confused at first because we already have a driver for lis3dh, but as an example on how to use Arduino drivers this is good to have.
Instead of including a copy of Adafruit_Sensor.h, can you create a separate pkg for the Adafruit_Sensor library that this pkg then depends on?

That would show how an Arduino driver can be used with minimal extra code.

@benpicco benpicco mentioned this pull request Jun 19, 2020
Merged
aabadie
aabadie reviewed Jun 23, 2020
View reviewed changes
pkg/arduino_lis3dh/Makefile Outdated
.PHONY: all

all:
"$(MAKE)" -C $(PKG_BUILDDIR) -f $(CURDIR)/Makefile.arduino_lis3dh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"$(MAKE)" -C $(PKG_BUILDDIR) -f $(CURDIR)/Makefile.arduino_lis3dh
"$(MAKE)" -C $(PKG_BUILDDIR) -f $(RIOTBASE)/Makefile.base MODULE=arduino_lis3dh

And you can remove the Makefile.arduino_lis3dh

aabadie
aabadie reviewed Jun 23, 2020
View reviewed changes
tests/arduino_lis3dh/Makefile Outdated
Comment on lines 7 to 8
USEMODULE += arduino_lib
USEMODULE += cpp11-compat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these dependencies could moved to the package dependencies.

aabadie
aabadie reviewed Jun 23, 2020
View reviewed changes
tests/arduino_lis3dh/Makefile Outdated
Comment on lines 9 to 10
USEMODULE += periph_i2c
USEMODULE += periph_spi
Copy link
Contributor

@aabadie aabadie Jun 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe conditionally add the right module to the build:

ARDUINO_LIS3DH_MODE ?= i2c

ifeq (i2c,$(ARDUINO_LIS3DH_MODE))
  FEATURES_REQUIRED += periph_i2c
else ifeq (spi,$(ARDUINO_LIS3DH_MODE))
  FEATURES_REQUIRED += periph_spi
  CFLAGS += -DARDUINO_LIS3DH_USE_SPI
endif

Copy link
Contributor Author

@gschorcht gschorcht Jan 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After trying a lot, it is not possible to define such dependencies without changing the original source completely by patching what is not the intention of package. The reason is that both interfaces are implemented by the driver which are not selected statically but by member values.

@aabadie
Copy link
Contributor

aabadie commented Jun 23, 2020

Instead of including a copy of Adafruit_Sensor.h, can you create a separate pkg for the Adafruit_Sensor library that this pkg then depends on?

+1

This package could have an empty all target and be declared as a pseudo-module (see gemmlowp as an example)

@gschorcht gschorcht force-pushed the pkg/arduino_lis3dh branch from 5e0b7d7 to c21e104 Compare June 25, 2020 10:18
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 11, 2020
@cgundogan cgundogan added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 14, 2020
@jdavid jdavid mentioned this pull request Oct 15, 2020
Closed
6 tasks
@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@gschorcht gschorcht mentioned this pull request Jan 2, 2022
Merged
@gschorcht gschorcht requested a review from cgundogan as a code owner January 2, 2022 15:40
@github-actions github-actions bot added Area: tools Area: Supplementary tools and removed Area: arduino API Area: Arduino wrapper API labels Jan 2, 2022
@gschorcht gschorcht force-pushed the pkg/arduino_lis3dh branch 2 times, most recently from 6516b75 to 33e62e1 Compare January 3, 2022 05:10
@gschorcht gschorcht mentioned this pull request Jan 3, 2022
Merged
@gschorcht gschorcht removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 3, 2022
@gschorcht gschorcht requested review from kYc0o and maribu as code owners January 3, 2022 17:17
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Area: sys Area: System Platform: AVR Platform: This PR/issue effects AVR-based platforms labels Jan 3, 2022
@github-actions github-actions bot removed Area: cpu Area: CPU/MCU ports Area: sys Area: System Platform: AVR Platform: This PR/issue effects AVR-based platforms labels Jan 3, 2022
@gschorcht
Copy link
Contributor Author

Instead of including a copy of Adafruit_Sensor.h, can you create a separate pkg for the Adafruit_Sensor library that this pkg then depends on?

+1

This package could have an empty all target and be declared as a pseudo-module (see gemmlowp as an example)

In difference to the gemmlowp package this package does not only provide header files and can be compiled.

@gschorcht
pkg/arduino_adafruit_sensor: add Adafruit Unified Sensor Driver
7aab412 
A number of Adafruit sensor drivers use the class Adafruit_Sensor from the Adafruit Unified Sensor Driver. To support such drivers, the Adafruit Unified Sensor Driver is imported as package.
@gschorcht
tests/pkg_arduino_adafruit_sensor: add test app
6b9690c
@gschorcht
pkg/arduino_lis3dh: Arduino library for LIS3DH
a8579be 
The library can be used for testing Arduino I2C and SPI interfaces.
@gschorcht
fixup! pkg/arduino_lis3dh: Arduino library for LIS3DH
822a248
@gschorcht
fixup! pkg/arduino_lis3dh: Arduino library for LIS3DH
2fc9a93
@gschorcht
fixup! sys/new_delete: add malloc/free based new/delete implementation
1ca6b75
@gschorcht gschorcht added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 18, 2022
@riot-ci
Copy link

riot-ci commented Dec 18, 2022

Murdock results

✔️ PASSED

1ca6b75 fixup! sys/new_delete: add malloc/free based new/delete implementation

Success Failures Total Runtime
2008 0 2008 03m:40s

Artifacts

bors bot added a commit that referenced this pull request Jan 15, 2023
@bors @gschorcht
Merge #17460
b8b1a60 
17460: pkg/arduino_adafruit_sensor: add Adafruit Unified Sensor Driver as package r=benpicco a=gschorcht

### Contribution description

This PR provides the [Adafruit Unified Sensor Driver](https://github.com/adafruit/Adafruit_Sensor) as package.

There are a number of Adafruit sensor drivers which all use a common base class `Adafruit_Sensor` from the [Adafruit Unified Sensor Driver](https://github.com/adafruit/Adafruit_Sensor). To support such drivers, the Adafruit Unified Sensor Driver is provided as package.

Adafruit sensor driver for ST LSM9DS0 will be provided as separat PR as package for demonstration and testing.

PR #12518 will be rebased for testing to have an Adafruit sensor driver for a sensor for which there is a native driver in RIOT.

### Testing procedure

Use a board that provides the `arduino` feature and flash
```
BOARD=... make -C tests/pkg_arduino_adafruit_sensor flash test
```
PR #12518 can be used as a more complex test for using the package.

### Issues/PRs references

Prerequisite for PR #12518

Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aabadie aabadie aabadie left review comments

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@smlng smlng Awaiting requested review from smlng

@cgundogan cgundogan Awaiting requested review from cgundogan

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

@kYc0o kYc0o Awaiting requested review from kYc0o

@maribu maribu Awaiting requested review from maribu

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss MrKevinWeiss is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports Area: tests Area: tests and testing framework Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR State: demonstrator State: This PR (loosely) demonstrates a concept and is not necessarily meant to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@gschorcht @benpicco @aabadie @riot-ci @cgundogan @smlng @MrKevinWeiss