avr: added EEPROM support for ATmega boards
This adds general EEPROM support for AVR devices through the EEPROM struct.
First MR here so I was hoping to get feedback on what I could improve and what I did wrong here.
It looks like it goes against convention to not have a Configure() function for hardware structures, but I did not see how It could make sense in this instance.
Also, should there be a specific EEPROM0 constant for each machine file that is then populated within the specific board file?
Thanks again, glad to be able to contribute to such a great project! :)
p.s. im an idiot, sorry about the other PR with the wrong branch selected :sweat_smile:
~~also now that im thinking about it, maybe the eepromSize constant should also be within board specific files?~~
edit: I have made changes to this commit so this comment does not apply anymore
I have a number of nits, but overall the code looks quite reasonable.
Can you also make a PR to https://github.com/tinygo-org/tinygo-site with API documentation on the EEPROM? At least the machine package, but if you could also write something in here (https://tinygo.org/docs/concepts/peripherals/) that would be great.
Absolutely. I'll make a write up once I fix the other issues you mentioned!
Sorry for the extreme delay on these changes, will go ahead and add them this week.
@SoleimanyBen I was looking at this PR today, and had a couple ideas. Seems to me that it would be great to implement the ReadWriteCloser interface for EEPROM as the main API developers would use.
Imaginary code:
func main() {
store := machine.EEPROM.Open(address)
defer store.Close()
var data [12]byte
_, err := store.Read(data)
if err != nil {
panic("nope")
}
println(string(data))
store.Write([]byte("Hola, mundo!))
}
@deadprogram That's a great idea! I'll write up those changes when I make the other adjustments later this week. :)
@SoleimanyBen I was looking at this PR today, and had a couple ideas. Seems to me that it would be great to implement the
ReadWriteCloserinterface for EEPROM as the main API developers would use.
Why though? It seems to me that ReaderAt and WriterAt are a much closer fit to the underlying hardware. With ReadWriteClose you have to:
- Open/close the EEPROM, which is not something the hardware needs
- Keep track of the read/write offset, which again isn't natively supported by the hardware
If an application really wanted to layer a ReadWriteCloser on top, you could easily write a new package that wraps the EEPROM to provide these methods. But for the machine API, I think it only adds unnecessary overhead.
@SoleimanyBen I think the interface to create here is actually the BlockDevice one, same as https://github.com/tinygo-org/tinyfs/blob/release/tinyfs.go#L42-L69
This is what I have been doing in my Flash PR #3472
@deadprogram EEPROM is not quite the same as flash though. Unlike flash, you can erase and rewrite individual bytes easily so "page size" (a big part of BlockDevice) doesn't make much sense.
In my opinion, this PR is almost ready, it just needs a few small changes to mergeable.