search

avr-rust / ruduino

Reusable components for the Arduino Uno.
Apache License 2.0
702 stars 50 forks source link

Make `Register::set` use `volatile_write` #31

Closed MalteT closed 3 years ago

MalteT commented 4 years ago

I was writing a small programs that uses the internal EEPROM of the ATmega328P, when I noticed that no writing was occuring. I was using

EECR::set(EECR::EEMPE);
EECR::set(EECR::EEPE);

to start writing to the EEPROM, but the memory was unchanged.

When I replaced the above code with the following:

EECR::write(EECR::EEMPE);
EECR::write(EECR::EEPE);

it worked like a charme.

I might be mistaken, but after digging through the Register code, I noticed that Register::set is not using volatile_write https://github.com/avr-rust/ruduino/blob/302095dca22f63a160d44c1fdcdc0fdf1db4072d/src/register.rs#L53-L57 while Register::unset, Register::write, etc are doing so. Is there a particular reason for this, that I may not understand yet? https://github.com/avr-rust/ruduino/blob/302095dca22f63a160d44c1fdcdc0fdf1db4072d/src/register.rs#L70-L74

Thanks!

laris commented 4 years ago

I also find the bug and you need to change the code.

dylanmckay commented 3 years ago

Indeed, every other method in this file was changed to use volatile memory operations, but this one method was fixed. I've given the file two passes over checking to see if there are any other nonvolatile memops on registers but this seems to be the only one. Now fixed.

Thanks for raising this!

I've released v0.2.7 which contains the fix for this issue.