Review Threads Operating Successfully #3

aclowmclaughlin · 2025-04-12T16:25:33Z

Initial RTOS thread setup- all the threads are at least templated out. Model thread is running.

…fy EVT-core, gonna need to make a branch there for stuff to actually work. CMake is having issues finding tx_api.h, which I thought I had resolved but apparently it is not happy about it still.

…ad to do anything.

…y CAN

DiegoLHendrix

Looks good. I just got a few grammatical nitpicks and some questions.

include/MCuC.hpp

include/PowertrainCAN.hpp

targets/REV3-Hardmon/main.cpp

targets/REV3-MCUC/main.cpp

DiegoLHendrix · 2025-05-25T09:50:11Z

targets/REV3-Hardmon/main.cpp

+[[noreturn]] void accessoryCanReceiveThreadEntry(accessoryCanReceiveThreadArgs_t* args) {
+    log::LOGGER.log(core::log::Logger::LogLevel::DEBUG, "Accessory Can Thread Started.");
+    rtos::TXError error;
+    //    while(true) {


I think I am missing something. Why is this while loop commented out?

The Hardmon dictionary is super incomplete (I was using the MCuC one to figure out what we needed) so running it will probably result in a crash, or at least undefined behaviour. Added a todo so it's more obvious what's wrong.

Is it safe to let a thread just exit like this without calling a thread delete or something? I would at least keep the loop and just put a suspend or long sleep in it to ensure it doesn't cause a problem.

It's generally safe, I believe that when a thread reaches the end of it's entry function it is set to sleep automatically. Technically it's likely not the best solution, but this won't be how it works in the actual code (we definitely need the accessory can thread), so it will be fine for now.

Yeah, but I was mostly thinking of leaving the loop and just sleeping for a really long time in it temporarily.

…nd cleaned up constructor in CPP.

mjh9585

Lots of nitpicky things but overall nice work. Have fun with this and I am sorry in advance. :)

.github/workflows/cmake.yml

include/Hardmon.hpp

mjh9585 · 2025-07-05T06:00:18Z

include/PowertrainCAN.hpp

     * Queue to store CAN messages.
     */
-    core::types::FixedQueue<POWERTRAIN_QUEUE_SIZE, IO::CANMessage> queue;
+    rtos::Queue queue;


This should probably be private.

PowertrainCAN is contained entirely within either MCuC or Hardmon, it's mostly just an easy way for me to keep all of those sorts of relevant messages together. Thus, the queue being public is probably better cause it's only access within the mcuc or hardmon, and PowertrainCAN is private to those classes.

Explaining this made me realize this is probably not the best way to do it- do you think that both classes should privately inherit from PowertrainCAN instead? That's probably the correct object-oriented way to do it.

I am not sure if inheritance is the right answer here since MCuC and Hardmon both use the accessory and powertrain busses. I was thinking some sort of access method would be better then inheritance. Then you can add any additional semephors before accessing the queue or preform any CAN state updates such as recording message time.

Bear in mind that MCuC and Hardmon are mutually exclusive in the sense that the target for MCuC and the target for Hardmon are both uploaded to entirely different chips.

include/MCuC.hpp

targets/REV3-MCUC/main.cpp

mjh9585 · 2025-07-05T08:46:09Z

targets/REV3-Hardmon/main.cpp

+    auto* queue = (rtos::Queue*) priv;
+    if (queue != nullptr) {
+        //TODO: determine if WaitForever is what we want to do in the interrupt- could be bad
+        queue->send(static_cast<void*>(&message), rtos::TXWait::TXW_WAIT_FOREVER);


You do not want to wait forever although I don't think it matters since you are in interrupt context and I believe all methods return immediately.

Also here in Hardmon you write directly to the Queue but in MCuC you write to the normal queue is there a reason for the difference?

Also also, you pass in a fixed queue but interpret it as a rtos queue.

Yeah the hardmon version was a little messed up. AccessoryCAN should be a fixedQueue.
I don't know what happens if a thread gets slept in the middle of an interrupt-- might be worth looking into.

targets/REV3-MCUC/main.cpp

mjh9585 · 2025-07-05T09:00:00Z

targets/REV3-Hardmon/main.cpp

+[[noreturn]] void accessoryCanReceiveThreadEntry(accessoryCanReceiveThreadArgs_t* args) {
+    log::LOGGER.log(core::log::Logger::LogLevel::DEBUG, "Accessory Can Thread Started.");
+    rtos::TXError error;
+    //    while(true) {


Is it safe to let a thread just exit like this without calling a thread delete or something? I would at least keep the loop and just put a suspend or long sleep in it to ensure it doesn't cause a problem.

…' into feature/aclowmclaughlin/VCU_RTOS

…and sendToPowertrainQueue() methods instead of just a getPowertrainQueue() method changed the targets (mains) of both to work with the new methods.

…' into feature/aclowmclaughlin/VCU_RTOS

…clude.

…th CAN library (the const gets dropped when sent)

…d of initializing them one by one. Also copied updated buffers for accessoryCAN from MCuC and Hardmon and changed corresponding methods.

…nd after the switch statement. Fixed typo in receiveFromPowertrainQueue (receive was spelled recieve) Switched the accessoryCanData_t to a union containing two structs in order to make memcpying separate parts of them more viable (then switched to using memcpy for both sendInputDataToSafeBuffer and sendOutputDataToUnsafeBuffer.

DiegoLHendrix

Good work

include/Hardmon.hpp

Co-authored-by: Diego <119289719+DiegoLHendrix@users.noreply.github.com>

tmb5932

Ok all I found that I was concerned (besides prev mentioned things) about are things ive already altered in my local version, so i think this is good.

I copied all the concerns/ideas that people (mainly heller) gave, so they are now part of my VCU todo list

aclowmclaughlin added 8 commits February 17, 2025 21:16

Initial Commit

fc965aa

Created Simulink Thread and PowertrainCAN Receive thread. Had to modi…

f4d5482

…fy EVT-core, gonna need to make a branch there for stuff to actually work. CMake is having issues finding tx_api.h, which I thought I had resolved but apparently it is not happy about it still.

random renaming fixes

a4bbc54

Updated MCuC target to use RTOS.

5c9d187

Stubbed out all Threads.

4b57297

Stubbed out all Threads.

3e272fa

Threads templating all done, needs CANopen for the Accessory CAN Thre…

66f6a5e

…ad to do anything.

Switched to different branch of EVT-core

89fd879

aclowmclaughlin requested a review from a team April 12, 2025 16:26

aclowmclaughlin and others added 16 commits April 12, 2025 13:16

Updated Github Actions

dbe10bf

Added PowerTrain Can Message IDs

d8ae947

whoopsies

57174d1

whoopsies part 2.

0cfa2a1

random commit.

caa2708

just need the build test to run again.

0cda216

updated evtcore

13d0c3d

updates so that MCuC can use AccessoryCAN

a9d1b2f

More Updates for Accessory CAN

688f79f

Fixed MCuC Object Dictionary.

8611ba9

updated MCuC_Model to new version.

6239099

set MCuC values to default false for safety.

ae6c7b0

Changes for Imagine stuff.

36a07c2

Changed back stuff that was messed up by imagine, fixed MCuC Accessor…

f9d05f3

…y CAN

updated to right evt-core branch version

90070d9

Applied Formatting Changes During GitHub Build

d870066

aclowmclaughlin requested a review from mjh9585 May 24, 2025 14:52

DiegoLHendrix self-requested a review May 25, 2025 05:57

DiegoLHendrix requested changes May 25, 2025

View reviewed changes

aclowmclaughlin requested a review from tmb5932 June 7, 2025 14:17

Fixed MCuC Imports, made a better description for the class itself, a…

56a686f

…nd cleaned up constructor in CPP.

aclowmclaughlin and others added 3 commits June 28, 2025 12:01

Added (Temporary) new pin array for testing. Will fix later

e7ee514

commented out incorrect statements

b010e91

Applied Formatting Changes During GitHub Build

a6c3b0e

mjh9585 requested changes Jul 5, 2025

View reviewed changes

aclowmclaughlin and others added 17 commits July 9, 2025 15:59

Changed cmake settings, updated clang format and github actions

c7db5dd

Merge remote-tracking branch 'origin/feature/aclowmclaughlin/VCU_RTOS…

00382fa

…' into feature/aclowmclaughlin/VCU_RTOS

fixed clang format

3776f57

Applied Formatting Changes During GitHub Build

417468c

Changed both MCuC and Hardmon to have a recieveFromPowertrainQueue() …

d836b5f

…and sendToPowertrainQueue() methods instead of just a getPowertrainQueue() method changed the targets (mains) of both to work with the new methods.

Applied Formatting Changes During GitHub Build

8767784

changed include in MCuC and removed unnecessary include in Hardmon.

6dfeb0b

Merge remote-tracking branch 'origin/feature/aclowmclaughlin/VCU_RTOS…

2c91881

…' into feature/aclowmclaughlin/VCU_RTOS

reordered PowertrainCAN includes and removed deprecated FixedQueue in…

efde182

…clude.

misc PowertrainCAN changes from Heller's PR feedback

8e348f2

reverted const changes to PowertrainCAN messages. didn't play well wi…

b179b66

…th CAN library (the const gets dropped when sent)

Applied Formatting Changes During GitHub Build

d931d73

Modified MCuC and Hardmon init() method to use bulk initialize instea…

afe3a4e

…d of initializing them one by one. Also copied updated buffers for accessoryCAN from MCuC and Hardmon and changed corresponding methods.

Applied Formatting Changes During GitHub Build

aa8e591

Applied Formatting Changes During GitHub Build

0637353

cleaned up commented out stuff.

148a2c2

DiegoLHendrix approved these changes Jul 17, 2025

View reviewed changes

include/Hardmon.hpp Outdated Show resolved Hide resolved

Diego Change.

a247cfc

Co-authored-by: Diego <119289719+DiegoLHendrix@users.noreply.github.com>

aclowmclaughlin requested a review from mjh9585 August 29, 2025 00:18

GitHub Build and others added 5 commits August 29, 2025 00:20

Applied Formatting Changes During GitHub Build

cf0694d

updated pins

fcf84a3

added todo's for documenting purposes

e97be61

updated naming and order of I/O pins

f6d9355

Applied Formatting Changes During GitHub Build

91ade96

tmb5932 approved these changes Oct 25, 2025

View reviewed changes

Review Threads Operating Successfully #3

Are you sure you want to change the base?

Review Threads Operating Successfully #3

Uh oh!

Conversation

aclowmclaughlin commented Apr 12, 2025

Uh oh!

DiegoLHendrix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjh9585 Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjh9585 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aclowmclaughlin Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DiegoLHendrix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tmb5932 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mjh9585 Jul 10, 2025 •

edited

Loading

aclowmclaughlin Jul 10, 2025 •

edited

Loading

tmb5932 left a comment •

edited

Loading