Skip to content

boards: Add support for the Arduino-Leonardo#11227

Merged
miri64 merged 7 commits intoRIOT-OS:masterfrom
maribu:leonardo
May 24, 2019
Merged

boards: Add support for the Arduino-Leonardo#11227
miri64 merged 7 commits intoRIOT-OS:masterfrom
maribu:leonardo

Conversation

@maribu
Copy link
Member

@maribu maribu commented Mar 21, 2019

Contribution description

  • Adds support for the ATmega32U4 cpu
  • Adds support for the Arduino Leonardo

Testing procedure

Try to flash and run some tests and examples, such as examples/hello-world. Keep in mind that most tests and examples require more resources (RAM & flash) than the Arduino Leonardo has.

Issues/PRs references

This PR is basically a rebase of #7306

@maribu maribu added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports labels Mar 21, 2019
@maribu maribu requested review from ZetaR60, kYc0o and miri64 March 21, 2019 13:45
@miri64
Copy link
Member

miri64 commented Apr 27, 2019

There are some differences between my branch (see #7306 (comment)) and this PR, so I need to retest. My branch also contains a lot of BOARD_INSUFFICIENT_MEMORY sets, so maybe this helps getting this through Murdock ;-).

@maribu
Copy link
Member Author

maribu commented Apr 27, 2019

There are some differences between my branch (see #7306 (comment)) and this PR, so I need to retest.

Sorry for the extra effort. Some refactoring of the AVR based boards has been merged since. Those required adaptations of the code to match current master.

My branch also contains a lot of BOARD_INSUFFICIENT_MEMORY sets, so maybe this helps getting this through Murdock ;-).

Thanks for the hint. I'll check that out. But if I remember correctly those adaptations have been done prior to the merge of the Arduino Nano - which needed to be added to BOARD_INSUFFICIENT_MEMORY for 99% of the apps and tests. I fear that your commit will no longer merge. I'll look into that once this PR is otherwise ready to merge, as a rebase mighy be required to update BOARD_INSUFFICIENT_MEMORY for new tests/apps merged since opening this PR.

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 23, 2019
@miri64
Copy link
Member

miri64 commented May 23, 2019

Sorry for testing only now, but I was quite busy the past month. examples/hello-world works, tests/periph_timer fails. This was a problem in #7306 as well which was later fixed.

@miri64 miri64 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 23, 2019
@maribu
Copy link
Member Author

maribu commented May 24, 2019

@miri64: Thanks for pointing that out. While I tried to deduplicate the timer configs of the ATmega1284P and the ATmega32U4 an error sneaked in (2 instead of 3 channels). I separated the configs now in the fix up, as this seems to be more readable.

I also changed the default frequency in tests/periph_timer for the Arduino Leonardo to use the same as for the other ATmega based Arduinos use. With that, the test now passes.

@miri64
Copy link
Member

miri64 commented May 24, 2019

I tested both examples/hello-world and tests/periph_timer again and now both work. Does it make sense to test I²C?

@maribu
Copy link
Member Author

maribu commented May 24, 2019

On the one hand the I2C driver is the same on all ATmega boards, on the other hand more testing is always better.

Due to memory constraints it will be a bit difficult to test. E.g. the i2c test overflows the memory. In this branch I rebased this PR against master (to get the i2c_scan shell command) and added the examples/i2c_scan as a minimal RIOT shell with only the commands reboot and i2c_scan provided. This is small enough to fit the Arduino Leonardo and allows very basic testing of the I2C interface.

@miri64
Copy link
Member

miri64 commented May 24, 2019

Tested I²C with tests/drivers_ds1307 (by attaching a ds1307 to the I²C pins first of course). Works 👍.

@maribu
Copy link
Member Author

maribu commented May 24, 2019

Perfect :-) Thanks :-)

Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

In summary: I tested on a linux system

  • examples/hello-world ☑️
  • tests/periph_timer ☑️
  • tests/drivers_ds1307 (with a DS1307 attached to the I²C pins) to confirm I²C capability ☑️

Please squash.

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 24, 2019
@miri64
Copy link
Member

miri64 commented May 24, 2019

Some builds already fail due to memory constraints

@maribu
Copy link
Member Author

maribu commented May 24, 2019

Yes, all the BOARD_INSUFFICIENT_MEMORY need to be extended. Will take about 30 minutes to do...

@miri64
Copy link
Member

miri64 commented May 24, 2019

Yes, all the BOARD_INSUFFICIENT_MEMORY need to be extended. Will take about 30 minutes to do...

sure. I remember the pain from #7306 ;-)

@maribu
Copy link
Member Author

maribu commented May 24, 2019

his branch has conflicts that must be resolved

F**k. (These are the BOARD_INSUFFICIENT_MEMORY updates). May I rebase?

@miri64
Copy link
Member

miri64 commented May 24, 2019

Of course!

tprrt and others added 3 commits May 24, 2019 15:12
The Arduino Leonardo requires - like the other ATmega based Arduinos - a
different frequency than the default 1000000, as this frequency cannot be
achieved on a 16MHz ATmega with any available prescaler.
@miri64
Copy link
Member

miri64 commented May 24, 2019

arduino-leonardo also needs to be blacklisted for 3 application (two because no malloc.h was found, one for because our sys/types.h for avrlibc defines time_t wrong [will fix]).

@miri64
Copy link
Member

miri64 commented May 24, 2019

Ok... apparently making time_t signed leads to problems avrlibc (see #8857). So won't fix.

@miri64
Copy link
Member

miri64 commented May 24, 2019

But for reference, in POSIX it shall be an integer type.

@miri64
Copy link
Member

miri64 commented May 24, 2019

@maribu you may squash. The blacklisting of the tests should be done in a separate commit anyways.

maribu added 2 commits May 24, 2019 17:23
Added arduino-leonardo to BOARD_INSUFFICIENT_MEMORY where needed
Added arduino-leonardo to BOARD_BLACKLIST where needed
@miri64
Copy link
Member

miri64 commented May 24, 2019

You may squash the missing blacklisting for tests/pipe immediately. Let's finally get this in!

maribu added 2 commits May 24, 2019 17:56
Added arduino-leonardo to BOARD_BLACKLIST where needed
Added arduino-leonardo to BOARD_INSUFFICIENT_MEMORY where needed
@miri64 miri64 merged commit 0f3ccb2 into RIOT-OS:master May 24, 2019
@miri64
Copy link
Member

miri64 commented May 24, 2019

🎉

@maribu
Copy link
Member Author

maribu commented May 24, 2019

@tprrt: Thank you very much for implementing the Arduino Leonardo support!

@miri64: Thanks for the review and for noticing my mess up with the timers :-)

@maribu
Copy link
Member Author

maribu commented May 24, 2019

@miri64:

But for reference, in POSIX it shall be an integer type.

I think that integer type does imply whether it is signed or unsigned. E.g. for short int, long int, long long int it is clear that they are signed, but char is also an integer type, but can either by signed or unsigned. (You can use signed char and unsigned char if you need to know what you get.)

@miri64
Copy link
Member

miri64 commented May 24, 2019

I think that integer type does imply whether it is signed or unsigned. E.g. for short int, long int, long long int it is clear that they are signed, but char is also an integer type, but can either by signed or unsigned. (You can use signed char and unsigned char if you need to know what you get.)

You are right. The value mktime() (and in turn timegm()) shall return is (time_t)-1, so it is actually minmea that needs a fix.

@miri64
Copy link
Member

miri64 commented May 24, 2019

Forgot the reference.

@miri64
Copy link
Member

miri64 commented May 24, 2019

Apparently this is fixed in the current version.

@maribu maribu deleted the leonardo branch May 24, 2019 17:38

# Modules to include:
USEMODULE += shell
USEMODULE += shell_commands
Copy link
Member

Choose a reason for hiding this comment

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

Why was this removed? oO

Copy link
Member

Choose a reason for hiding this comment

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

Fixed in #11599

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no idea how that sneaked in. I did touch about every example and test Makefile to update the BOARD_INSUFFICIENT_MEMORY, so I guess I just hit a wrong key and lost track of the changes :-(

Copy link
Member

Choose a reason for hiding this comment

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

Me as well... Shit happens ;-).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: boards Area: Board ports Area: cpu Area: CPU/MCU 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.

3 participants