Merge datastorage framework in to main #12
Labels
No Label
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Mikrokontrollersystemer-gruppe-4/mikrokontrollersystemer-prosjekt#12
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "eeprom"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
closes #7
LGTM OwO
Needs a couple changes.
@ -0,0 +1,98 @@#include "eeprom.h"// The start address for the controller datauint8_t EEMEM startAddressController = 0x00;Can these be #defines in the header file?
They feel like they should be defines in the header file.
No, they can't. The EEMEM does not allow it do be as defined, and it does not like being in the header.
@ -0,0 +16,4 @@while(1){if (eeprom_is_ready()){break;printf("it is");Can this printf be reached if it is behind break?
If it is dead code it should be removed.
Fixed in
4a8b6137d1@ -0,0 +19,4 @@printf("it is");}else{;printf("its not");Printing every cycle feels redundant. Remove?
Fixed in
4a8b6137d1@ -0,0 +30,4 @@// Checks if it has been written information at the address// If true, the infromation is replaced with the intaken struct// else the intaken struct is written at the address.void WriteStructInEEPROM(config_t writeStruct){Function names should use a standardized naming scheme.
I have used
word_another()so far, what about you @Inamr ?ina used word_another(), I wil be changing the standarized naming sheme
Fixed in
4a8b6137d1@ -0,0 +32,4 @@// else the intaken struct is written at the address.void WriteStructInEEPROM(config_t writeStruct){uint8_t structSize;structSize = sizeof(writeStruct);This should be assigned on the previous line.
@ -0,0 +40,4 @@}// Reads the memory block at the address 0x00This comment should not say 0x00, it should name the variable that it actually uses. Even if that is 0x00 at this moment that might change, which would make the comment invalid.
Fixed in commit
a8be04b583@ -0,0 +42,4 @@// Reads the memory block at the address 0x00// returns a struct in form of config_tconfig_t ReadStructInEEPROM(){Read/WriteStructInEEPROM()functions should probably be calledRead/WriteStructFromEEPROM()for proper english.fixed in
a8be04b583@ -0,0 +57,4 @@// checks if EEPROM is ready// If the dataset is 1, the datapoint is written at the address.// If the dataset is 2 its written at another point.void WriteDataPointInEEPROM(uint8_t dataPoint, uint8_t data){what is dataPoint? is dataPoint a random byte? If so call it byte.
The data variable should probably be called
fan_numor something. As it is it is not clear what data is.The way I read this initially was the data was the data to prite and that dataPoint was maybe some pointer to where to write it.
Fixed in commit
a8be04b583@ -0,0 +62,4 @@checkEEpromIsReady();if (data == 1){eeprom_update_byte(currentAddressFan1, dataPoint);currentAddressFan1++;Why are these global? Could they not be a local volatile variable?
i agree but not shure if eemem allows it, will check
i need the variables in write and read function so i do not think it should be local, if you do not have a solution to that problem?
@ -0,0 +67,4 @@eeprom_update_byte(currentAddressFan2, dataPoint);currentAddressFan2++;} else{// error???A common way to return errors is to make the function an int.
An int returning 0 = OK. Anyting other than 0 indicates and error.
If you want to return an error this is the way to do it.
@ -0,0 +74,4 @@// Reads all the datapoints to the choosen data.// it writes the data points in the USART stream.void ReadDataPointSpeedInfo(uint8_t data){This should return a pointer to the start of an array and the length of said array.
This allows us to do what we want to the data, not just print it.
Alternatively a function can be made later to output it over I2C, but returning a pointer and a length is a lot more flexible.
@ -0,0 +1,50 @@/** File: eeprom.h* Author: athamantisThis should be your actual name, not your username.
Fixed in commit
a8be04b583@ -4,15 +4,29 @@** Created on March 6, 2024, 12:34 PM*//*We should not have two header sections.
Add your name to the list instead.
Fixed in commit
a8be04b583@ -10,0 +18,4 @@#include <stdint.h>#define F_CPU 4E6#include <util/delay.h>#include <avr/eeprom.h>Is this needed in main? If unused it should be removed.
Fixed in commit
a8be04b583@ -10,0 +20,4 @@#include <util/delay.h>#include <avr/eeprom.h>#include <stdbool.h>#define USART3_BAUD_RATE(BAUD_RATE) ((float)(F_CPU * 64 / (16 *(float) BAUD_RATE)) + 0.5)Remove.
Fixed in commit
a8be04b583LGTM
Pull request closed