Hi all! I’m glad to see the community is so active. When I bought this a few days ago I thought the community was dying.
I just started programming for the Arduboy and this is my first (released) app! Its got zoom in, zoom out, clear, and I’m working on adding other features. Make sure to leave me feedback so I can improve 
Very cool … I like the zoom in and out.
A couple of little issues:
Great! maybe outputting the image via serial could be a feature
Another suggestion - scrolling left / right and up / down.
It ebs and flows.
Naturally it’s probably going to be more active towards the weekend.
EEPROM:
Firstly, @filmote’s right about the EEPROM, I’m not sure what you’re trying to do with it, but it looks like you’re trying to use it as scratch space rather than for saving, which is a bad idea because it’ll burn the EEPROM out quicker.
EEPROM has a limited number of times you can write to it, which is quite a lot to be fair, but it’ll still wear out quicker if you’re not careful about how often you read and write to it.
Also update is preferred over write because it will avoid writing if the data at the location is already the same value.
And of course, I have no idea why you’re putting two related variables 512 bytes apart from each other.
And px and py are int, which is 2 bytes wide, but you’re only storing them into EEPROM as 1 byte, which aside from anything else means they’ll never be -1 because read returns a uint8_t which is unsigned, so when it’s cast to int it’ll only ever become a value between 0 and 255 inclusive, which means your if (px != -1 && py != -1) check will always be false (though I doubt the compiler knows this, so the resulting code is probably doing the check anyway).
Object Duplication:
I like that you’ve put some of your code in a separate class.
You’ve made a fairly significant mistake though: you’ve ended up with two instances of the Arduboy2 class because you’ve got one inside your ArduArt class and one as a global variable.
In C++, objects are copied by value by default, and that’s what happening here. I think what you wanted to do was make a reference (and possibly you’re used to a language where that would be a reference, like Java or C#), which in C++ requires using either & (a reference) or * (a pointer).
In this case, the easiest solution would be a reference:
class ArduArt
{
Arduboy2 & arduboy;
public:
ArduArt(Arduboy2 & n_arduboy)
{
arduboy = n_arduboy;
}
// etc
};
Otherwise the only issues I can see are stylistic issues - stuff that could be improved but nothing that’s actually a bug.
(If I get chance to run it I might spot something else.)
Hi! Thanks for all the suggestions.
So the reason I’m using the EEPROM instead of the flash is because I began to run out of ram while placing pixels. The reason I place the x and y 512 apart is so that I have the first for the x, and the second for the y. The reason I did this was because with the delete button, it needs to look for where the cursor is and refer to it’s storage point, which wouldn’t work in pairs because it would be impossible to find later (IE [1,1], [4,9], etc). I’ll definitely implement the update feature, as well as I’m working on having the screen load on boot.
Also, how many writes so you think the storage has? It makes me think twice about using the EEPROM in this way.
Edit: I’ll also use your reference method. That was what I was trying to do.
Also, it checks for -1 -1 so that when an object is “deleted” it’s coords are set to -1. And then when draw _art() goes through EEPROM to see which ones are valid (=! -1) then it will draw. It saves ram. But if EEPROM only stores -1 then what’s happening? Because the delete feature works.
It would work. If it didn’t work multidimensional arrays wouldn’t work.
The technique for using 2 dimensions to index a 1 dimensional structure is typically called ‘flat arrays’.
The equation for the indexing is: y * width + x.
Say you had a 16x16 array of bytes.
If you wanted to access (0, 4), that’s 0 * 16 + 4, which is the byte at index 4.
If you wanted to access (3, 5) that’s 3 * 16 + 5, the byte at index 53.
As long as x and y are valid indices (i.e. between 0 and width - 1 for x and between 0 and height - 1 for y) then it’ll work fine.
Yeah, your intent was fairly clear, which is a good thing.
For more about references, there’s a reasonably good introductory article here:
https://www.cprogramming.com/tutorial/references.html
Out of interest, is C++ your first language or did you learn something else first?
As I said, in many other languages that would have been valid, but it’s not in C++ because of C++'s philosophy of giving the programmer greater control at the expense of requiring them to know more to do things effectively.
It’s in the datasheet for the 32u4 chip.
100,000 writes. It seems like a lot, but as I say that depends how often you’re writing to it. You’re more likely to burn through the flash than the EEPROM, but it’s still not designed to be some kind of RAM replacement.
It doesn’t store -1, that’s the point.
An int is a 2-byte signed value. EEPROM.read returns a uint8_t, a 1-byte unsigned value. When a uint8_t is implicitly converted to an int, it’s treated as an unsigned value, so it will only convert to a value between 0 and 255 inclusive.
So here:
int px = EEPROM.read(EEPROM_STORAGE_SPACE_START + i);
int py = EEPROM.read(EEPROM_STORAGE_SPACE_START + 512 + i);
if (px != -1 && py != -1) {
arduboy.fillRect(px * TILE_SIZE, py * TILE_SIZE, TILE_SIZE, TILE_SIZE, WHITE);
}
px and py will never be -1 (I misspoke earlier and said the condition would be "always false" instead of "always true").
Here’s some code as proof:
#include <Arduboy2.h>
Arduboy2 arduboy;
int x;
int y;
void setup()
{
arduboy.begin();
// Write -1, pretend i = 0
EEPROM.write(EEPROM_STORAGE_SPACE_START, -1);
EEPROM.write(EEPROM_STORAGE_SPACE_START + 512, -1);
// Set x and y to prove there's no magic
x = 0;
y = 0;
// Read "-1", pretend i = 0
x = EEPROM.read(EEPROM_STORAGE_SPACE_START);
y = EEPROM.read(EEPROM_STORAGE_SPACE_START + 512);
// Note I'm only writing and reading once at the start to avoid wearing out the EERPOM
}
void loop()
{
if(arduboy.nextFrame())
return;
arduboy.clear();
// lo and behold, 255
arduboy.print("x :");
arduboy.println(x);
arduboy.print("y :");
arduboy.println(y);
arduboy.display();
}
This isn’t true. This does nothing to save RAM.
Even if the code worked as intended, the whole frame buffer still exists wether the rectangle is drawn or not.
Not drawing the rectangle may save time (and thus processing power), but it doesn’t save RAM.
Got chance to go over it some more.
Most of the things I spotted were just stylistic things, but two things ocurred to me.
Firstly you’re only using about half the available RAM.
Secondly you have a 20 element array and you’re only using two of the entries and I can see no discernable reason why.
So that’s 36 bytes wasted there.
Even so, I can’t see you’d be breaking the RAM threshold unless you’re trying to store a ridiculously large number of points.
In which case using EEPROM is only going to buy you a few hundred points even when used properly, it’s only got 1024 bytes to begin with (16 of which are ‘eaten’ by the Arduboy2 library).
I did a fairly significant rewrite Getting it down to 9526 bytes of progmem and 1556 bytes of RAM.
This also took away the save functionality, but that can be added easily enough.
I won’t post it unless you ask, just in case you would rather figure stuff out on your own or make some more attempts of your own first, but if you’re curious the offer’s there.
That is right. BUT, “wearing out” means that it cannot store things for like 10 years (which is what it is supposed to do) and will instead die after maybe two years. And after a very large amount (millions) of writing time the entire thing DIEs, and recovery is not possible. But you are probably rocking another Arduboy after it started to really wear out after like, a century.
EEPROM have NOTHING to do with RAM.
BUT if you knows how to solder things well, you can replace that EEPROM chip-just solder a new one one won’t make too big a mess. I guess…
But if its avoidable why would you do this anyway?
Also If i remember correctly the EEPROM is integrated into the Arduino chipset
@Dreamer2345 Makes an excellent point.
The EEPROM is not replaceable because it’s actually built into the processor, hence why it’s listed on the ATmega 16U4/32U4 datasheet.
The datasheet states:
- 512Bytes/1KB Internal EEPROM
- Write/Erase Cycles: 10,000 Flash/100,000 EEPROM
- Data retention: 20 years at 85 C/ 100 years at 25 C
You’d have to replace the whole processor, which wouldn’t be particularly easy.
(And personally I’d like to make my Arduboy last anyway.)
I don’t mind if you post it. This was my first program and it seems I messed up pretty bad, so at least it could be fixed by someone who knows more about it. Plus, I could learn what I did wrong. Thanks!
For a first program, I’ve seen a lot worse.
I could understand what most of your code was doing, which is always a good thing. Some people are capable of writing programs that do amazing things but end up with nearly illegible code.
The by-value instead of by-reference thing is completely expected,
some people get quite far into C++ without knowing about references.
The EEPROM problem is the only thing I’d call a serious bug because it’s the only thing that actually has a negative impact. 99% of bugs can’t actually harm the console in any way, this is one of those extremely rare cases where there were non-obvious implications that could cause harm (although still in a fairly limited way).
Essentially you didn’t “mess-up pretty bad”, you made a beginners mistake, you just got a bit unlucky as to which mistake that was.
I’ve forked your repo and uploaded all my changes to my fork in a single commit.
I’ve commented all the changes I made to try to explain why I made them.
Rather than following your naming conventions and brace style, I followed my own preferred style, partly because I can write faster that way, partly so it’ll stand out more.
Some of my changes are changes that are ‘subjectively better’ (i.e. other programmers might disagree). When it comes to programming, some things are clear cut and others are eternally debated.
I’m prepared to explain every single change if you have any questions.
Also note that I didn’t put saving functionality in because I wasn’t sure which button combos you’d want for saving and loading or whether you were planning to add some kind of menu for that.
Saving is fairly easy though, you’d just have to loop through all the points and write them to EEPROM using EEPROM.put like so:
Edit: Almost forgot EEPROM_STORAGE_SPACE_START.
(Though ideally you should offset the address a bit. A lot of games write straight to EEPROM_STORAGE_SPACE_START and as a side effect it causes that area of EEPROM to wear out faster than later areas because of all the games picking that area.)
for(size_t i = 0, addr = EEPROM_STORAGE_SPACE_START; i < ArduArt::maxPoints; ++i, addr += sizeof(Point2))
{
EEPROM.put(addr, arduart.points[i]);
}
Edit: or (subjectively more readable), this version:
size_t addr = EEPROM_STORAGE_SPACE_START;
for(size_t i = 0; i < ArduArt::maxPoints; ++i)
{
EEPROM.put(addr, arduart.points[i]);
addr += sizeof(Point2);
}
And EEPROM.get can do the reverse.
You could improve that by saving the number of points before actually saving the points, so instead of checking for a special value (i.e. -1), you could just limit the number of points you load (which would also be slightly faster).
I’ll leave you to have a go at doing that yourself first.
Like I said, your code isn’t bad, it’s quite well written.
You’re just lacking knowledge, and nobody can blame you for that.