Skip to content

Conversation

keestux
Copy link
Contributor

@keestux keestux commented Oct 7, 2019

Contribution description

This PR adds the Arduino include files for 4 Sodaq boards (Autonomo, Explorer, One, Sara AFF).

As an extra this PR also adds the ADC configuration for Sodaq Autonomo. Furthermore it adjusts the ADC configurations for the other Sodaq boards so that each of them can properly reads it BATVOLT.

Testing procedure

Build and run tests/periph_gpio_arduino and tests/sys_arduino.

@keestux keestux requested a review from aabadie October 8, 2019 17:58
@miri64 miri64 added Area: arduino API Area: Arduino wrapper API Area: boards Area: Board ports Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Oct 8, 2019
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

The CI is reporting trailing whitespaces. Otherwise the changes look good (even if not verified with the datasheet but I tend to trust you @keestux :) ).
I won't be able to test this PR on my Sodaq boards before next week unfortunately. Maybe just provide some output ?

@keestux
Copy link
Contributor Author

keestux commented Oct 8, 2019

@aabadie the only thing I did was to use the toggle command in tests/periph_gpio_arduino to switch on/off the LEDs.
When #12180 comes through there will be more ways to test. Not that I want to wait for that.

For ADC I just used tests/periph_adc. I think that all Sodaq boards have BATVOLT at the last ADC channel. The value should be somewhat stable when a battery is attached.

@keestux keestux force-pushed the sodaq-boards-arduino branch from eba2937 to 413087c Compare October 8, 2019 21:09
@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 9, 2019
aabadie
aabadie previously approved these changes Oct 9, 2019
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Code changes look good. I trust @keestux tests.

ACK

@aabadie aabadie dismissed their stale review October 9, 2019 07:25

Murdock found problems and there are indeed issues.

@aabadie
Copy link
Contributor

aabadie commented Oct 9, 2019

Murdock is reporting an issue with tests/driver_hd44780.

Looking a second time into this PR, it seems arduino_pinmap.h is missing for all boards.

@keestux
Copy link
Contributor Author

keestux commented Oct 9, 2019

I'll have a look. My first impression is that this is a weird test to be part of RIOT.

@keestux
Copy link
Contributor Author

keestux commented Oct 9, 2019

Also adding @smlng because he added this test.

This test (tests/driver_hd44780) is the only thing in RIOT that actually uses ARDUINO_PIN_*. Why did we let that in? The test can only be used on one specific board with a particular pinout. There is no universal agreement in the Arduino world about the function of the pins. Pin2 on one arduino board can be totally different from pin2 on another board.

From the README of the test

  • Pin 4 (RS or "register select") is connected to pin 2 on the Arduino
  • Pin 6 (EN or "enable") is connected to pin 3 on the Arduino.
  • Pin 11 on the LCD is connected to pin 4 on the Arduino.
  • Pin 12 on the LCD is connected to pin 5 on the Arduino.
  • Pin 13 on the LCD is connected to pin 6 on the Arduino.
  • Pin 14 on the LCD is connected to pin 7 on the Arduino.

It does not even say which Arduino, is it Arduino Zero?

Can't we pin the test to just that board?

Right now the gain and reference are hard coded. Until it is configurable
at runtime prefer the same default as with Arduino Core. That is,
GAIN_DIV2 and REFSEL_INTVCC1. This way we can properly measure the
battery voltage.
The Sodaq boards support Arduino, but there are no ARDUINO_PIN_* defines.
@keestux keestux force-pushed the sodaq-boards-arduino branch from 3594a13 to 57463d7 Compare October 9, 2019 18:19
@keestux
Copy link
Contributor Author

keestux commented Oct 9, 2019

@aabadie For now I have added my boards to the black list of that test. Murdock is happy.

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Blacklisting the boards in the test application is ok: the same strategy was done for stm32f4discovery.
Still trusting @keestux tests and cde changes are good.

ACK

@aabadie aabadie merged commit 9bc600a into RIOT-OS:master Oct 14, 2019
@keestux
Copy link
Contributor Author

keestux commented Oct 14, 2019

Thanks

@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
@keestux keestux deleted the sodaq-boards-arduino branch October 21, 2022 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: arduino API Area: Arduino wrapper API Area: boards Area: Board ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants