-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ADBMS Integration #138
base: develop
Are you sure you want to change the base?
ADBMS Integration #138
Conversation
…ed on pulling voltages
int DataIdx; | ||
|
||
for (DataIdx = 0; DataIdx < len; DataIdx++) { | ||
__io_putchar( *ptr++ ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
messed up diff this stuff was changed on main??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this cuz printing was broken and i think it was cuz of the usart dma changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm did u use \r
? Also make sure to use ner serial
or else it wont work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bunch of random stuff idk you prob know all of this anyway as this is WIP.
Only thing actually wierd is the whole wakeup IC stuff and how we now have a bunch of different types of wakeup functions all seeming to do similar stuff.
Core/Inc/bmsConfig.h
Outdated
#define NUM_CELLS_PER_CHIP 10 | ||
#define NUM_SEGMENTS 1 | ||
#define NUM_CHIPS 1 //NUM_SEGMENTS * 2 | ||
#define NUM_CELLS_PER_CHIP 16 | ||
#define NUM_THERMS_PER_CHIP 32 | ||
#define NUM_RELEVANT_THERMS 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its the number of therms that we actually wanted to read on a segment. I think we'll be getting rid of this but i think its outside the scope of this.
Core/Src/segment.c
Outdated
#define TOTAL_IC 1 | ||
cell_asic IC[TOTAL_IC]; | ||
|
||
RD REDUNDANT_MEASUREMENT = RD_OFF; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do want redundant in final rev
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure we can do that later. I want a full spec from BMS team on exactly what the configs should be and how they want the BMS to work
Core/Src/segment.c
Outdated
cell_asic IC[TOTAL_IC]; | ||
|
||
RD REDUNDANT_MEASUREMENT = RD_OFF; | ||
CH AUX_CH_TO_CONVERT = AUX_ALL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to setup which AUX channel is read by which multiplexer. The mtg notes I think say the main multiplexer reads internal register and the AUX is dedicated to the GPIOs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These variables won't make it into the final draft. this was copied over from adbms source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They'll be gone once this code is tested.
void get_c_adc_voltages(cell_asic *chip) | ||
{ | ||
wake_and_write_configs(chip); | ||
adbms_wake(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why wake so many times?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats just what worked in testing
int DataIdx; | ||
|
||
for (DataIdx = 0; DataIdx < len; DataIdx++) { | ||
__io_putchar( *ptr++ ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm did u use \r
? Also make sure to use ner serial
or else it wont work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more code hip hip hooray. really its impossible to figure out what is good and bad practice until the high level code gels better (we remove the years of therm and algo disable crap).
Core/Src/segment.c
Outdated
} | ||
// TODO: Its kind of silly to just copy voltages like this. We should consolidate | ||
// chip data and chips later, or design analyzer to read voltages from chips rather | ||
// than chipdata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree with both of the above TODOs.
*/ | ||
void read_aux2_registers(cell_asic chips[NUM_CHIPS]) | ||
{ | ||
write_config_regs(chips); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for all read_* commands. Why do we write to registers each time? Feels like we dont need to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the chip slept, all the config registers are wiped (I think).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this is from the adbms example and has worked in testing so we're keeping it for now. Agreed that we should investigate this though.
Changes
Low level drivers and some integration.
Test Cases
test bench
To Do
Checklist
It can be helpful to check the
Checks
andFiles changed
tabs.Please reach out to your Project Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
Closes #133 (issue #)