Skip to content

Conversation

mali
Copy link
Contributor

@mali mali commented May 18, 2016

Add basic support of :
- atmega328p MCU
- arduino uno board

@kaspar030 kaspar030 added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label May 18, 2016
/*
* Copyright (C) 2014 Freie Universität Berlin, Hinnerk van Bruinehsen
* 2015 Kaspar Schleiser <kaspar@schleiser.de>
* 2016 Laurent Navet <laurent.navet@gmail.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation off

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@OlegHahm OlegHahm added the Platform: AVR Platform: This PR/issue effects AVR-based platforms label May 18, 2016
UART0_RX_IRQ_EN;
break;
#endif /* UART_0_EN */
#if UART_1_EN
Copy link
Contributor

Choose a reason for hiding this comment

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

There's only one UART on this CPU, so the code referring to uart interfaces > 0 should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@aabadie
Copy link
Contributor

aabadie commented May 19, 2016

Tested on an arduino uno, works if one change the PROGRAMMER variable to arduino. The stk500v2 seems to work for the duemilanove board which is similar to the Uno.
Maybe someone has idea for elegantly solving this issue ?

Apart from that and the comment about the UART interfaces, this is a good job, thanks !

@miri64
Copy link
Member

miri64 commented May 19, 2016

Maybe someone has idea for elegantly solving this issue ?

Do it similar to the cortex-m platforms: make an (or multiple aptly named) arduino_common directory(s) and add only the bare minimum changes for arduino-uno or arduino-duemilanove to the respective directories.

export DEBUGGER_FLAGS = "-x $(RIOTBOARD)/$(BOARD)/dist/gdb.conf $(ELFFILE)"
export DEBUGGER = $(DIST_PATH)/debug.sh $(DEBUGSERVER_FLAGS) $(DIST_PATH) $(DEBUGSERVER_PORT)

# PROGRAMMER defaults to stk500v2 which is the internal flasher via USB. Can be
Copy link
Contributor

Choose a reason for hiding this comment

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

programmer needs to be updated in the comment as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed, fixed

@kYc0o kYc0o added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label May 20, 2016
@mali
Copy link
Contributor Author

mali commented May 20, 2016

Squashing Done.

@kYc0o
Copy link
Contributor

kYc0o commented May 21, 2016

Actually the squashing should be done when the review is done and the PR is ready to be merged. I put the label just to don't forget to do it... sorry if it made you think that the squashing was needed now...

@mali
Copy link
Contributor Author

mali commented May 21, 2016

No problem, will squash again if needed :)

@mali
Copy link
Contributor Author

mali commented May 25, 2016

I've added a patch that place shared code between atmega328p and atmega2560 in atmega_common.
except startup.c that certainly can be shared but I haven't found how yet (build fails for what I've tried now)

@kYc0o kYc0o added this to the Release 2016.07 milestone May 30, 2016
@mali
Copy link
Contributor Author

mali commented Jun 2, 2016

Started a wiki page : https://github.com/RIOT-OS/RIOT/wiki/Board:-Arduino-Uno

@kYc0o kYc0o added the State: waiting for other PR State: The PR requires another PR to be merged first label Jun 7, 2016
@kYc0o
Copy link
Contributor

kYc0o commented Jun 7, 2016

Let's wait for #5529, it will simplify the code since only few modifications to the common sources will be needed.

@PeterKietzmann
Copy link
Member

@mali #5529 was merged. Did you plan to implement a small port for the duemilanove as well?

@mali
Copy link
Contributor Author

mali commented Jun 22, 2016

I can if you're interested, at least the duemilanove with the atmega328.
I don't have the atmega168 model.

@PeterKietzmann
Copy link
Member

I don't want to force you. But I've got one duemilanove (atmega328) laying around which I never used before :-)

@mali
Copy link
Contributor Author

mali commented Jun 23, 2016

For what I know the only difference between the two is that the Duemilanove uses a FTDI USB chip while the Uno uses a second Atmel who acts as USB-serial converter. So that should be simple

@kYc0o
Copy link
Contributor

kYc0o commented Jun 29, 2016

Just looked at the latest commits and you still have the cpu/atmega328p/periph folder, which is no more necessary, since all the peripherals work equally for all atmega CPUs. Can you remove it?

add atmega328p support with:
uart, timer, spi and gpio
@mali mali changed the title boards/arduino-uno : add basic support boards: add arduino uno and duemilanove support Sep 5, 2016
@mali
Copy link
Contributor Author

mali commented Sep 5, 2016

@PeterKietzmann, you can remove dust from your duemilanove and test it :-)

@mali
Copy link
Contributor Author

mali commented Sep 10, 2016

a video proof that it works.
A timer pulse which toggles gpio.

arduno uno riot os support

@mali
Copy link
Contributor Author

mali commented Sep 19, 2016

I think all issues have been fixed.
Please let me know if you wait for something more before 2016.10 release.

@aabadie aabadie added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable State: waiting for other PR State: The PR requires another PR to be merged first labels Sep 19, 2016
@aabadie
Copy link
Contributor

aabadie commented Sep 19, 2016

@mali. Just tested with an Arduino Uno (I don't have duemilanove) : it works fine.

I give an ACK. Let's now wait for Murdock.

@aabadie
Copy link
Contributor

aabadie commented Sep 19, 2016

@mali, just in case you missed it, Murdock is complaining (search for "Errors" in the page pointed by "Details" above).
There are a few trailing whitespaces and a warning generated by doxygen.
To fix the coap test error, you have to add the arduino-uno/duemilanove to the BOARD_BLACKLIST list (see test/coap/Makefile fir example).
The remaining cortex_m4_2 and cortex_m3_1 errors don't seem to be related.

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.

@mali with the inline comment, doxygen warning should be fixed.
But I don't really understand what's wrong with the pipe test build failure (malloc.h missing). Maybe @kaspar030 has an idea ?

*/

/**
* @defgroup boards_arduino-common
Copy link
Contributor

Choose a reason for hiding this comment

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

The doxygen warning triggering a Murdock error comes from this line (try make doc at the root of riot repository and look at the first warning).
To fix it, you can change it like this:

* @defgroup    boards_arduino-common Arduino common

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thank's !

@mali
Copy link
Contributor Author

mali commented Sep 20, 2016

Murdock is green :-)

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.

2 things remaining:

  • apply changes requested by the comment above
  • squash the commits

After that => ACK (again)

@@ -3,6 +3,8 @@ include ../Makefile.tests_common

BOARD_BLACKLIST := arduino-mega2560 waspmote-pro
# arduino-mega2560: unknown type name: clockid_t
BOARD_BLACKLIST += arduino-uno arduino-duemilanove
Copy link
Contributor

Choose a reason for hiding this comment

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

arduino-uno and arduino-duemilanove boards are blacklisted for the same reason (unknown type clockid_t) so no need to add those 2 extra lines, you can just append them to the first line and update the comment above.
There are a other places were this should be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Uno and Duemilanove(atmega328p version) are nearly the same boards.
The only difference is that the Duemilanove use an FTDI usb chip,
while the Uno use an Atmel which acts as USB/Serial converter.
All of the code needed to support these boards is in arduino-common.
- blacklist arduino-uno and arduino-duemilanove for
  coap, libfixmath_unittests, lwip, nhdp,
  pthread, pthread_barrier, pthread_cleanup, pthread_condition_variable
  pthread_cooperation, pthread_rwlock and pthread_tls tests.

- fix sys/pipe build

- unittests: boards added to BOARD_INSUFICIENT_MEMORY list.
@mali
Copy link
Contributor Author

mali commented Sep 21, 2016

New failures comes from #5763 merge

Copy link
Contributor Author

@mali mali left a comment

Choose a reason for hiding this comment

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

@kYc0o or @LudwigKnuepfer I would like your opinion on this change related to PR5763

@aabadie
Copy link
Contributor

aabadie commented Sep 21, 2016

Sorry @mali, I missed your previous comment. And looking at #5763, I think this PR is now potentially broken because of this line.
You now need to add something similar to this in board.h for arduino-uno/duemilanove if I understand well.

@aabadie
Copy link
Contributor

aabadie commented Sep 21, 2016

Sorry again, @mali. You already backported the change and fixed the failing CI ! Great :)
I'll try again this branch tomorrow.

@mali
Copy link
Contributor Author

mali commented Sep 22, 2016

@aabadie , I had this in queue in another branch ;-)

@aabadie
Copy link
Contributor

aabadie commented Sep 22, 2016

Just tested it with some examples and tests. It works.

ACK and go !

@aabadie aabadie merged commit b2b42c7 into RIOT-OS:master Sep 22, 2016
@mali mali deleted the uno branch September 22, 2016 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: AVR Platform: This PR/issue effects AVR-based platforms 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.

10 participants