Skip to content

Implement CCU Soft Review Feedback#407

Open
captainwerty wants to merge 23 commits intomainfrom
CCU_Review
Open

Implement CCU Soft Review Feedback#407
captainwerty wants to merge 23 commits intomainfrom
CCU_Review

Conversation

@captainwerty
Copy link
Copy Markdown
Contributor

Problem and Scope

Description

Gotchas and Limitations

Testing

  • HOOTL testing
  • HITL testing
  • Human tested

Testing Details

Larger Impact

Additional Context and Ticket

Copy link
Copy Markdown
Contributor

@dchansen06 dchansen06 left a comment

Choose a reason for hiding this comment

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

Small change of when the debug message is printed to no longer be inside the interrupt

Please also fill in the PR description

LOGOMATIC("PRECHARGE TS ACTIVE: %d\n", state_data.BCU_PRECHARGE_SET_TS_ACTIVE);

LOGOMATIC("====================================\n\n");
VCP_StateDump(&state_data);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You cannot do this inside of an interrupt (the ISR) it just takes too much time. Anything done in an ISR supercedes CAN interrupts (which are the lowest priority interrupt due to the risk of the bus flooding and nothing happening) and printing is slow and in an ISR it cannot be interrupted to capture a message or something, while outside of an ISR it can be interrupted just fine.

Instead, I want you to define a global volatile boolean and here I want you to set that flag high; then, on state tick, I want you to check the status of the flag and if it is high then I would like you to call the function to print all the things and set the flag low.


Something vaguely like the following perhaps

// In StateUtils.h
extern volatile bool request_print_statedata;
// In StateUtils.c
volatile bool request_print_statedata;

void CheckForDebugPrint(void)
{
    if (!request_print_statedata)
        return;

    LOGOMATIC(...)
    ...
    LOGOMATIC(...)

    request_print_statedata = false;
}
Suggested change
VCP_StateDump(&state_data);
request_print_statedata = true;

And then just call CheckForDebugPrint() inside of the main infinite while loop

}
}

void VCP_StateDump(const CCU_StateData *state_data)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dchansen06 dchansen06 marked this pull request as draft April 10, 2026 06:59
@dchansen06 dchansen06 changed the title CCU Review Implement CCU Soft Review Feedback Apr 10, 2026
@dchansen06 dchansen06 added Enhancement New feature or request 2 PRIORITY Important and a priority, but less than URGENT labels Apr 10, 2026
@dchansen06 dchansen06 added Boards Related to or involving any physical boards Bug Something is doing a thing but doing it wrong or otherwise incorrectly labels Apr 10, 2026
@dwilson765 dwilson765 marked this pull request as ready for review April 10, 2026 19:25
Copy link
Copy Markdown
Contributor

@dchansen06 dchansen06 left a comment

Choose a reason for hiding this comment

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

A few code duplication problems, and a few changing 0/1 to proper false/true for boolean values

@dchansen06 dchansen06 marked this pull request as draft April 10, 2026 20:18
@dwilson765 dwilson765 marked this pull request as ready for review April 11, 2026 07:41
dchansen06
dchansen06 previously approved these changes Apr 12, 2026
Copy link
Copy Markdown
Contributor

@dchansen06 dchansen06 left a comment

Choose a reason for hiding this comment

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

Lgtm

@dchansen06 dchansen06 dismissed their stale review April 12, 2026 01:42

Pending PR description

@dchansen06 dchansen06 self-requested a review April 12, 2026 01:43
Copy link
Copy Markdown
Contributor

@dchansen06 dchansen06 left a comment

Choose a reason for hiding this comment

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

Please fill in the PR description

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

Labels

2 PRIORITY Important and a priority, but less than URGENT Boards Related to or involving any physical boards Bug Something is doing a thing but doing it wrong or otherwise incorrectly Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants