OnionIoT / Onion-Docs

Documentation for all things Onion
https://docs.onion.io
GNU General Public License v3.0
113 stars 65 forks source link

OLED-Expansion-C-Library.md #11

Open ID-Rocketeer opened 7 years ago

ID-Rocketeer commented 7 years ago

uint8_t *buffer = malloc(OLED_EXP_WIDTH*OLED_EXP_HEIGHT/8 * sizeof *buffer); // allocate memory for the buffer

This statement appears twice in the file. It should be either

uint8_t *buffer = malloc(OLED_EXP_WIDTH*OLED_EXP_HEIGHT/(8 * sizeof *buffer)); // allocate memory for the buffer

or

uint8_t *buffer = malloc(OLED_EXP_WIDTH*OLED_EXP_HEIGHT/8 / sizeof *buffer); // allocate memory for the buffer

so that the number of elements allocated decreases as the size of the data type used for buffer increases. The only reason to include "sizeof *buffer" in the calculation is if you're intent is to make the code more bullet-proof against changes in the base data type, so the code should work correctly if that is done.

greenbreakfast commented 7 years ago

The end goal is to allocate 1024 bytes, since the display is controlled by programming 8 vertical pixels with a single byte. So to get to 1024 bytes, I divide the total number of pixels (128x64) by 8, and then multiply it by the size of the first element of the buffer pointer which is uint8_t. I always want to allocate 1024 elements. But should I not be using sizeof *buffer?

ID-Rocketeer commented 7 years ago

The point is that you need to divide by the size of the buffer element, not multiply. Because your buffer element is 1-byte you get the result you expect, but if you were to change the type of the buffer to an array of uint32_t you'd end up with 4096 members, not 256.