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

Add support for programming AVR EEPROM memory if .eeprom section foun… #403

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cr1901
Copy link
Contributor

@cr1901 cr1901 commented Mar 30, 2023

…d in ELF file.

Tested locally and working. This is meant to complement a set of crates I'm working on...

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.

Hi, thanks, this is a very good addition. The only thing I am thinking to change is whether we should provide a commandline parameter to explicitly disable EEPROM writing. I am thinking about use-cases where settings are stored in EEPROM and you want the option to not overwrite them on most uploads (yet still provide the .eeprom section for defaults). What's your take on this?

@Rahix Rahix added the ravedude label Mar 31, 2023
@cr1901
Copy link
Contributor Author

cr1901 commented Mar 31, 2023

What's your take on this?

Sure, I can add a flag to disable EEPROM flashing.

I think most Rust cases using the .eeprom section would want the EEPROM to be reflashed if e.g. cargo triggered a rebuild of the crate(s) providing an .eeprom section. But I can envision use cases where a Rust firmware is provided an .eeprom section as part of an C or asm object file/library, and you don't necessarily want to reflash the EEPROM every time.

@cr1901
Copy link
Contributor Author

cr1901 commented Apr 2, 2023

Please do not merge yet, I believe this patch is incorrect/imprecise (It's not .eeprom section name that's special. Rather, I think avrdude is checking the ELF's program headers for data for references to the EEPROM address space).

I'll work on fixing it in the coming days.

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

Successfully merging this pull request may close these issues.

2 participants