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.

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

[DO NOT MERGE YET] Fix HID Bootloader #77

Closed
wants to merge 4 commits into from

Conversation

NicoHood
Copy link
Contributor

Fixes #76

The clock divider is not essential, but I just added it as in the other bootloader sketch.

You could compact the code a bit more and more the check upwards and save the reg before. I just wanted to keep an better overview though.

See this for more information:
http://www.nongnu.org/avr-libc/user-manual/group__avr__watchdog.html

Edit:
Do not merge this PR yet. I will update a few more things and comment when its ready.

@NicoHood NicoHood changed the title Fix HID Bootloader [DO NOT MERGE YET] Fix HID Bootloader Mar 29, 2016
@abcminiuser
Copy link
Owner

Fixed in f0c5dfe.

uint32_t PageAddress = ((uint32_t)Endpoint_Read_16_LE() << 8);
#else
uint16_t PageAddress = Endpoint_Read_16_LE();
#endif

/* Check if the command is a program page command, or a start application command */
#if (FLASHEND > 0xFFFF)
#if (FLASHEND > USHRT_MAX)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abcminiuser Why didnt you pickup this line of fix?

Copy link
Owner

Choose a reason for hiding this comment

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

They're equivalent, no? I prefer the explicit constant since the constraint isn't the type but the device's actual memory layout, but I suppose the named value works just 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.

Yes, just keep it. Its fine and should be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants