Skip to content

add write block w/o length, renaming read_char#222

Merged
DocMoebiuz merged 4 commits intoMobiFlight:mainfrom
elral:EEPROM_optimization
Dec 8, 2022
Merged

add write block w/o length, renaming read_char#222
DocMoebiuz merged 4 commits intoMobiFlight:mainfrom
elral:EEPROM_optimization

Conversation

@elral
Copy link
Copy Markdown
Collaborator

@elral elral commented Nov 23, 2022

read_char() is renamed to read_byte() as mainly a byte is required.
Within OnGetInfo() a char is required to send the config via the command messenger, at this point read_byte() is casted.
Additional functions to save/read a complete array or struct. This could be helpfull for future use (e.g. parameters for an analog calibration should be in a struct and this struct should be saved to the EEPROM). These additional functions and the existing functions for reading/saving arrays with fixed lenght are moved to EEPROM.h in a template.

storeName() and restoreName() are cleaned up slightly

Fixes #221

@elral elral requested a review from DocMoebiuz as a code owner November 23, 2022 14:28
@github-actions
Copy link
Copy Markdown

Firmware for this pull request:
Firmware.zip

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 8, 2022

Firmware for this pull request:
Firmware.zip

Copy link
Copy Markdown
Collaborator

@DocMoebiuz DocMoebiuz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TY!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 8, 2022

Firmware for this pull request:
Firmware.zip

@DocMoebiuz DocMoebiuz merged commit b1e0ddb into MobiFlight:main Dec 8, 2022
@GioCC
Copy link
Copy Markdown
Contributor

GioCC commented Dec 12, 2022

Not sure about the benefit of templating these functions.
I'm afraid that the compiler might generate redundant instances of the same function (unless we trust it to be smart enough - which I admit it might well be) if the template is instantiated for different types.
I see that even the basic EEPROM.get() uses templating - but it's still a framework library, so redefining it would get in the way of porting.
Passing by reference has no particular purpose other than mask the fact that we are getting its raw pointer anyway. What's bad in plainly invoking the basic functions with & and sizeof()?
Besides, defining the templates in the .h file rather than in the .cpp should inline them - is that desired?

(BTW, casting to uint8_t* would not work for non-POD objects - which we are not currently using and won't probably be using anytime soon, but still something to be aware of.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EEPROM optimisation

3 participants