boards/feather-m0: add arduino feature

[jdavid]

Contribution description

Adds the arduino feature to the feather-m0 board.

The pins come from https://github.com/adafruit/ArduinoCore-samd/tree/master/variants/feather_m0

Testing procedure

I've tested with tests/periph_gpio_arduino and tests/sys_arduino_lib. Both work.

But examples/arduino_hello-world and tests/sys_arduino don't work, Serial does not print anything.
That's why I'm creating this PR as a draft.
Any help understanding why Serial doesn't work with the feather-m0 is much appreciated.

Issues/PRs references

None.

[gschorcht]

The pin mapping is a bit difficult since not all Arduino pins are available as external pins. Some of them are used as internal control signals for additional hardware, e.g., for the WiFi module, the LoRa modul or the BLE module, others doesn't exist at all.

There are several ways to solve this problem:

1. All the pins that are not exposed as external pins are undefined (GPIO_UNDEF) to keep the order.
2. Only the pins that are not used are defined as undefined (GPIO_UNDEF), pins that are used internally are defined to keep the order and the compatibility with existing Arduino libraries that control additional hardware.
3. Pins are reordered to have at least the 13 digital pins as defined by Arduino Uno.

Furthermore, you have redefined a number of pins. Why?

[gschorcht]

See comment above.

[gschorcht]

D2...D4, D7, D8 can't be used as external pins. They could be defined as GPIO_UNDEF.

Another possible solution would be to try to use a pinout that differs from the official one to define at least all the pins that exist on Arduino Uno. For example, on Arduino Uni D11...D13 are used for SPI. So you could define the pins that are used for SPI on Feather M0 (your D22..D24) as D10...D13 and use your D10...D13 as D2...D4.

D4, D7, D8 are used on different Feather M0 variants to control additional hardware like the WiFi module, the BLE module and the LoRa module. They could be defined even though they are not exposed to keep compatibility with existing Arduino libraries.

[gschorcht]

Doesn't exist and can't be used as ADC channel.

Suggested change

#define ARDUINO_PIN_A6 ARDUINO_PIN_8 /**< A6 */

[gschorcht]

Suggested change

#define ARDUINO_A6 ADC_LINE(6) /**< ADC 6 */

The same here.

[gschorcht]

Suggested change

	#define ARDUINO_PIN_25 GPIO_PIN(PB, 3) /**< D25 */
	#define ARDUINO_PIN_26 GPIO_PIN(PA, 27) /**< D26 */
	#define ARDUINO_PIN_27 GPIO_PIN(PA, 28) /**< D27 */

These pins are for an additional UART interface that is not broken out (AFAIK).

[gschorcht]

Suggested change

#define ARDUINO_PIN_38 GPIO_PIN(PA, 13) /**< D38 */

Not connected

[gschorcht]

Suggested change

	#define ARDUINO_PIN_32 GPIO_PIN(PA, 22) /**< D32 */
	#define ARDUINO_PIN_33 GPIO_PIN(PA, 23) /**< D33 */
	#define ARDUINO_PIN_34 GPIO_PIN(PA, 19) /**< D34 */
	#define ARDUINO_PIN_35 GPIO_PIN(PA, 16) /**< D35 */
	#define ARDUINO_PIN_36 GPIO_PIN(PA, 18) /**< D36 */
	#define ARDUINO_PIN_37 GPIO_PIN(PA, 17) /**< D37 */

Redefinition of D20 (SDA), D21 (SCL), D12, D11, D10, D13.

[gschorcht]

Suggested change

	#define ARDUINO_PIN_39 GPIO_PIN(PA, 21) /**< D39 */
	#define ARDUINO_PIN_40 GPIO_PIN(PA, 6) /**< D40 */
	#define ARDUINO_PIN_41 GPIO_PIN(PA, 7) /**< D41 */

Redefinition of D7, D8, D9

[gschorcht]

Suggested change

#define ARDUINO_PIN_42 GPIO_PIN(PA, 3) /**< D42 */

AREF pin.

[gschorcht]

Suggested change

	#define ARDUINO_PIN_43 GPIO_PIN(PA, 2) /**< D43 */
	#define ARDUINO_PIN_44 GPIO_PIN(PA, 6) /**< D44 */
	#define ARDUINO_PIN_45 GPIO_PIN(PA, 7) /**< D45 */

Redefinition of A0, D8, D9

[gschorcht]

But examples/arduino_hello-world and tests/sys_arduino don't work, Serial does not print anything. That's why I'm creating this PR as a draft. Any help understanding why Serial doesn't work with the feather-m0 is much appreciated.

I don't have the hardware with me, I can take a look next week.

[aabadie]

Any help understanding why Serial doesn't work with the feather-m0 is much appreciated.

In RIOT, this board is defined to use stdio over USB but at the moment the Arduino port can only work with stdio over UART. That's why it doesn't work.

RIOT/sys/arduino/serialport.cpp

Lines 47 to 52 in 2e0c02b

	void SerialPort::begin(long baudrate)
	{
	/* this clears the contents of the ringbuffer... */
	ringbuffer_init(&rx_buf, rx_mem, SERIAL_RX_BUFSIZE);
	uart_init(dev, (uint32_t)baudrate, rx_cb, (void *)&rx_buf);
	}

To make Serial work on feather-m0 (but also on the arduino-mkr boards), one has to adapt the Arduino port to work with stdio over USB.

[jdavid]

Pins updated.

Is the stdio issue blocking this PR? Since it doesn't work for the mkr boards either, maybe it can be done in a different PR?

[jdavid]

I've looked at the Serial problem.

This is not finished, but I'd like to gather feedback, to know whether I'm heading in the right direction.
See the latest commit, with it Serial works with the feather-m0 board.

Basically it implements Serial on top of https://doc.riot-os.org/group__sys__stdio.html
To do that I added a couple of functions (stdio_avail and stdio_clear), but for now they're
only implemented in sys/usb/usbus/cdc/acm/cdc_acm_stdio.c

There are a few issues though, aside from the missing implementations of the new functions.
Like what to do with the baudrate parameter in SerialPort::begin(..)

[gschorcht]

This works only, if all stdio_* backends implement this function. std_uart does not at the moment.

[gschorcht]

By the way, this would be an API change and should at least be done in a separate PR. An API change should only be made if and when absolutely necessary. It would require that all stdio_* backends implement it.

[gschorcht]

In case of std_uart backend, the baudrate is fixed and defined by STDIO_UART_BAUDRATE.

[gschorcht]

In case of stdio_uart backend, this requires to enable module stdio_uart_rx to work.

[gschorcht]

In case of stdio_uart, it might be enough to call stdio_init. It depends on periph_uart implementation.

[gschorcht]

I'm not sure whether it is necessary to clear buffers here at all.

[gschorcht]

By the way, this would be an API change and should at least be done in a separate PR. An API change should only be made if and when absolutely necessary. It would require that all stdio_* backends implement it.

[gschorcht]

stdio_init is called during CPU initialization and should only be called once. Calling it again here might lead to problems. It should be sufficient to initialize members of the SerialPort object or variables of the module here if there are any.

[gschorcht]

I'm not sure whether it is necessary to clear buffers here at all.

[gschorcht]

This could be realized without extending the stdio_base API by using a ringbuffer as for direct used of periph_uart. In that case Serial::begin would clear the ringbuffer and stdio_base wouldn't be necessary.

[gschorcht]

This is not finished, but I'd like to gather feedback, to know whether I'm heading in the right direction.

One question is whether we can rely on that Serial object in Arduino always means the interface which is used for stdio in RIOT (Arduino serial interfaces). Another question is how the signature of the constructor of Serial should look like. At the moment, it gets a UART_DEV defined ARDUINO_UART_DEV.

[aabadie]

IMHO, the change related to Serial should be in its own PR as it affects more boards (also Arduino mkr and maybe others).

Similar to PWM and the assiocated arduino_pwm, maybe it's worth to have an arduino_serial module ?
I agree with @gschorcht that it's important to keep the possibility to use other UART port with Serial, to remain compatible with the Arduino API.

[gschorcht]

Maybe, a possible solution could be to use different Serial implentations dpendent on slected modules that use different interfaces, for example

void SerialPort::begin(long baudrate)
{
#if IS_USED(MODULE_STDIO_CDC_ACM)
    ...
#else
    /* this clears the contents of the ringbuffer... */
    ringbuffer_init(&rx_buf, rx_mem, SERIAL_RX_BUFSIZE);
    uart_init(dev, (uint32_t)baudrate, rx_cb, (void *)&rx_buf);
#endif
}

or something like that. The question here would be whether it should be possible to use a specified UART_DEV even though module stdio_cdc_acm is used.

In that case, a possibility could be :

void SerialPort::begin(long baudrate)
{
    if (dev != UART_UNDEF) {
        /* this clears the contents of the ringbuffer... */
        ringbuffer_init(&rx_buf, rx_mem, SERIAL_RX_BUFSIZE);
        uart_init(dev, (uint32_t)baudrate, rx_cb, (void *)&rx_buf);
    }
    else {
         ... /* stdio case */
    }
}

This would make it possible to use an interface UART if defined or stdio if not. The board can control it by overriding the definition of ARDUINO_UART_DEV.

[jdavid]

Ok thanks. I'll close this PR and open a new one only for the feather-m0.
And later open another PR for serial.