Skip to content

Conversation

@badeniran
Copy link
Member

@badeniran badeniran commented Nov 28, 2020

Reviews to review logic and code. Will begin functional testing in Winter 2021. The pull request will remain open until we know that the new aux BMS is functional.

The design is based on these two documents:
Aux BMS Architecture V2 - Proposal
Aux BMS Architecture V2 - Diagram

badeniran added 23 commits July 25, 2020 14:34
We should double check DMA, ADC and SPI settings
Only difficulty is that for CMSIS v2, CubeMx does not support memory pools so we are assuming
we'll be fine with just queues. Worst case, we'll use pvMalloc and pvFree for dynamic allocation.
…ine-and-initialize-variables

User/embedded/sft 167 define and initialize variables
Created functions, tasks, and unit tests relating to contactor control.
…ate-functions-and-unit-tests-relating-to-contactor-control

User/embedded/sft 175 create functions and unit tests relating to contactor control
…peripheral-configs-and-initializations

CAN Interrupt and Filter Config
…ate-functions-and-unit-test-relating-to-orion-work

User/embedded/sft 190 create functions and unit test relating to orion work
…200-Create-functions-relating-to-CAN-Tx-work
…ate-functions-relating-to-CAN-Tx-work

User/embedded/sft 200 create functions relating to can tx work
…CSolarCarTeam/Epsilon-Embedded-Software into user/embedded/SFT-202-Creating-functions-and-unit-tests-relating-to-aux-voltage-reading-work
…ating-functions-and-unit-tests-relating-to-aux-voltage-reading-work

User/embedded/sft 202 creating functions and unit tests relating to aux voltage reading work
…ating-functions-and-unit-tests-relating-to-aux-voltage-reading-work

Adding unit tests
Implemented stack overflow hook, and malloc failed hook, and error handler
…lement-error-handling

SFT 203. Implementing error handling
…lement-memory-debug-tasks

SFT 204. Implemented memory debug task
@duan-le
Copy link
Contributor

duan-le commented Mar 13, 2021

@bill-luu, @badeniran please take a look when you are free

Copy link
Member Author

@badeniran badeniran left a comment

Choose a reason for hiding this comment

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

Looking good! Just a few changes and questions

AuxBmsContactorState expectedAuxBmsContactorState = {.contactorsDisconnected = 1};
uint32_t contactorControlEventFlags = 0x28; // CHARGE_OPENED and DISCHARGE_OPENED

osMessageQueueGet_ExpectAndReturn(orionInterfaceQueue, &message, NULL, ORION_QUEUE_TIMEOUT, osErrorTimeout);
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if you meant for this to return osErrorTimeout

Copy link
Contributor

@duan-le duan-le Mar 14, 2021

Choose a reason for hiding this comment

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

if (status == osErrorTimeout) // If waiting on the queue times out
{
    //Set orion can message recieved recently to 0
    localAuxStatus.orionCanReceivedRecently = 0;
}
else
{
    // Determine trip conditions and contactor settings based on Orion CAN
    localAuxStatus.orionCanReceivedRecently = 1;
    updateAllowChargeAndAllowDischarge(message, &localAuxStatus);
    updateAuxTrip(message, &localAuxTrip);
    localAuxStatus.dischargeShouldTrip = checkDischargeTrip(localAuxTrip);
    localAuxStatus.chargeShouldTrip = checkChargeTrip(localAuxTrip);
    shouldDisconnectContactors = localAuxTrip.protectionTrip
                                || localAuxStatus.dischargeShouldTrip
                                || localAuxStatus.chargeShouldTrip;
}

We were not able to test this else block because of the localAuxTrip being a local variable. That's why we were returning osErrorTimeout. Not sure if there is a way to test this particular block, but we did test most of these functions in TripTest.c.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can think of two ways:

  • Don't mock the Trip functions. This will make testing incredibly messy
  • Make localAuxTrip not local to the function. This is less clean in the actual design code, but cleaner for testing

Either way, I think it's very critical to test this point as this has all the trip functionality.
@bill-luu what do you think?

Copy link
Member Author

@badeniran badeniran left a comment

Choose a reason for hiding this comment

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

Looking good. Just a comment on the one test. We can wait to see what @bill-luu has to say on that as well

AuxBmsContactorState expectedAuxBmsContactorState = {.contactorsDisconnected = 1};
uint32_t contactorControlEventFlags = 0x28; // CHARGE_OPENED and DISCHARGE_OPENED

osMessageQueueGet_ExpectAndReturn(orionInterfaceQueue, &message, NULL, ORION_QUEUE_TIMEOUT, osErrorTimeout);
Copy link
Member Author

Choose a reason for hiding this comment

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

I can think of two ways:

  • Don't mock the Trip functions. This will make testing incredibly messy
  • Make localAuxTrip not local to the function. This is less clean in the actual design code, but cleaner for testing

Either way, I think it's very critical to test this point as this has all the trip functionality.
@bill-luu what do you think?

Charge will cause a trip due to:
* the max cell voltage being higher than the Orion limit
* high temperature while charging
* discharge pack current being too high (absolute value)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be charge right?

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.

5 participants