Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

atmega-hal: mcu support for atmega2560 #138

Merged
merged 6 commits into from
Feb 15, 2021
Merged

atmega-hal: mcu support for atmega2560 #138

merged 6 commits into from
Feb 15, 2021

Conversation

explicite
Copy link
Contributor

@explicite explicite commented Feb 13, 2021 •
edited
Loading

atmega2560 mcu support like 356090d

Rahix
Rahix reviewed Feb 14, 2021
View reviewed changes
Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

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

Thanks a lot for starting work on this! I already took a quick look over your code and left a few review comments. Overall, everything looks right though. Thanks!

mcu/atmega-hal/src/lib.rs Show resolved Hide resolved
mcu/atmega-hal/src/lib.rs Show resolved Hide resolved
mcu/atmega-hal/src/usart.rs Outdated Show resolved Hide resolved
mcu/atmega-hal/src/usart.rs Outdated Show resolved Hide resolved
@Rahix
Copy link
Owner

Rahix commented Feb 14, 2021

BTW, I would suggest doing each of the three "steps" as individual pull-requests if that's all right with you?

@explicite @Rahix
Update mcu/atmega-hal/src/usart.rs
61a4efe 
Co-authored-by: Rahix <rahix@rahix.de>
@explicite explicite changed the title atmega-hal: Add support for atmega2560 atmega-hal: mcu support for atmega2560 Feb 14, 2021
@explicite explicite marked this pull request as ready for review February 14, 2021 19:42
@explicite
Copy link
Contributor Author

Scope limited to MCU impl only. Thanks @Rahix for the review :)

Rahix
Rahix requested changes Feb 14, 2021
View reviewed changes
Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

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

One minor issue, it doesn't compile for me... Left a comment with the necessary fix. If you apply that, I think we're go for merging.

I am curious though why it was working for you... Or did you never try compiling? ;)

mcu/atmega-hal/src/usart.rs Outdated Show resolved Hide resolved
@explicite @Rahix
Update mcu/atmega-hal/src/usart.rs
fa83037 
Co-authored-by: Rahix <rahix@rahix.de>
@explicite
Copy link
Contributor Author

I am curious though why it was working for you... Or did you never try compiling? ;)

@Rahix I run cargo in different patch - I'm using master for comparison XD
Let me fix this and then will ask you for review. Sorry for that

@Rahix Rahix self-requested a review February 15, 2021 08:23
@Rahix Rahix added the mcu-support Support for a new Microcontroller label Feb 15, 2021
@Rahix Rahix added this to the New APIs milestone Feb 15, 2021
@Rahix Rahix merged commit 8cffcc8 into Rahix:next Feb 15, 2021
@Rahix Rahix mentioned this pull request Feb 16, 2021
Open
42 tasks
@explicite explicite deleted the atmega-hal/mega2560 branch February 25, 2021 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rahix Rahix Rahix approved these changes

Assignees
No one assigned
Labels
mcu-support Support for a new Microcontroller
Projects
None yet
Milestone
New APIs
Development

Successfully merging this pull request may close these issues.
2 participants
@explicite @Rahix