Here is a patch to implement EXT for arduino mega2560.
The implementation was made with chibios-svn-9835-trunk.
If you apply the patch to a trunk tree of ChibiOS, the following will be done:
- Modify some file (os/hal/include/hal_ext.h and os/hal/ports/AVR/platform.mk)
- Creation of the EXT driver low device driver in os/hal/ports/AVR
- Creation of a test directory under testhal/AVR/. This is an application to test the driver.
A lot of optimisations can be done maybe?
If there is a better way to do things don't hesitated to share!
It will be nice to have some feedback on the code style and the application execution.
I will be happy to improve if nedded.
Giovanni, Utzig, I also need your help to see if driver design rules are respected so that this can be part of the AVR port?
Any feedback are welcome.
[PATCH] AVR EXT Topic is solved
- tfAteba
- Posts: 547
- Joined: Fri Oct 16, 2015 11:03 pm
- Location: Strasbourg, France
- Has thanked: 91 times
- Been thanked: 48 times
[PATCH] AVR EXT
- Attachments
-
- 0001-ext-arduino-mega2560.patch.zip
- (15.35 KiB) Downloaded 445 times
regards,
Theo.
Theo.
-
- Posts: 359
- Joined: Sat Jan 07, 2012 6:22 pm
- Location: Brazil
- Has thanked: 1 time
- Been thanked: 20 times
- Contact:
Re: [PATCH] AVR EXT
Hi,
Is this new code or is it based on the changes Igor had written on his github repo?
Is this new code or is it based on the changes Igor had written on his github repo?
-
- Posts: 359
- Joined: Sat Jan 07, 2012 6:22 pm
- Location: Brazil
- Has thanked: 1 time
- Been thanked: 20 times
- Contact:
Re: [PATCH] AVR EXT
In general the patch looks ok. If you could format it to follow the ChibiOS/RT style that would be helpful, specifically never using tabs, using two spaces for indentation, one space after keywords like if and while and the open parenthesis, one space after the opening { at line ends.
Also you should not use sei and cli but instead some form of osalSysLock/Unlock*.
And also you can attribute the copyright to yourself if you would prefer that!
Cheers,
Fabio Utzig
Also you should not use sei and cli but instead some form of osalSysLock/Unlock*.
And also you can attribute the copyright to yourself if you would prefer that!
Cheers,
Fabio Utzig
- Giovanni
- Site Admin
- Posts: 14457
- Joined: Wed May 27, 2009 8:48 am
- Location: Salerno, Italy
- Has thanked: 1076 times
- Been thanked: 922 times
- Contact:
Re: [PATCH] AVR EXT
Correct, everything HAL-related can be under copyright of the authors as long the Apache 2.0 license is kept (in order to have a single license to deal with).
Another note, templates files are under my copyright but that can be changed, probably we should remove my name there and just leave an XXXXXXXX inside the license text.
I also "strongly encourage" to take coding rules seriously
Giovanni
Another note, templates files are under my copyright but that can be changed, probably we should remove my name there and just leave an XXXXXXXX inside the license text.
I also "strongly encourage" to take coding rules seriously
Giovanni
- tfAteba
- Posts: 547
- Joined: Fri Oct 16, 2015 11:03 pm
- Location: Strasbourg, France
- Has thanked: 91 times
- Been thanked: 48 times
Re: [PATCH] AVR EXT
Thank you for your prescision.
Note that for the code style I was not aware of.
I will submit another patch to see if every things is correct!
Note that for the code style I was not aware of.
I will submit another patch to see if every things is correct!
regards,
Theo.
Theo.
- tfAteba
- Posts: 547
- Joined: Fri Oct 16, 2015 11:03 pm
- Location: Strasbourg, France
- Has thanked: 91 times
- Been thanked: 48 times
Re: [PATCH] AVR EXT
Hi all,
Utzig, here is the patch formated by following ChibiOS style .
I have also attribute the copyright to myself for the following three files:
Let me know is there is still another important point to review.
Utzig, here is the patch formated by following ChibiOS style .
I have also attribute the copyright to myself for the following three files:
- - os/hal/ports/AVR/hal_ext_lld.c
- os/hal/ports/AVR/hal_ext_lld.h
- testhal/AVR/EXT/main.c
Let me know is there is still another important point to review.
- Attachments
-
- 0002-ext-arduino-mega2560.patch.zip
- (15.21 KiB) Downloaded 438 times
regards,
Theo.
Theo.
-
- Posts: 359
- Joined: Sat Jan 07, 2012 6:22 pm
- Location: Brazil
- Has thanked: 1 time
- Been thanked: 20 times
- Contact:
Re: [PATCH] AVR EXT Topic is solved
Thanks, applied!
Btw, you added a new define for a new ext mode and ended up not using it. I would suppose it's the else condition in ext_lld_set_intx_edges. I left it there anyway.
Cheers,
Fabio Utzig
Btw, you added a new define for a new ext mode
Code: Select all
#define EXT_CH_MODE_LOW_LEVEL 5U /**< @brief low level callback. */
Cheers,
Fabio Utzig
- tfAteba
- Posts: 547
- Joined: Fri Oct 16, 2015 11:03 pm
- Location: Strasbourg, France
- Has thanked: 91 times
- Been thanked: 48 times
Re: [PATCH] AVR EXT
The Mega has 4 trigger modes, falling, rising, both and low level mode.
The else if of course for the low level mode(EXT_CH_MODE_LOW_LEVEL). May be I should use it explicitly, just want to know for next time.
Thanks.
The else if of course for the low level mode(EXT_CH_MODE_LOW_LEVEL). May be I should use it explicitly, just want to know for next time.
Thanks.
regards,
Theo.
Theo.
Who is online
Users browsing this forum: No registered users and 29 guests