Skip to content
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

Update main.c #361

Merged
merged 35 commits into from
Jan 2, 2024
Merged

Update main.c #361

merged 35 commits into from
Jan 2, 2024

Conversation

Cam0Cow
Copy link
Contributor

@Cam0Cow Cam0Cow commented Sep 30, 2023

Quality Assurance Checklist

To make reviews more efficient, please make sure the software feature meets the following standards and check everything off that meets the quality check. Once everything has been checked, the assigned reviewers will begin the review process. Edit this description to check off the list.

There are exceptions with all guidelines. As long as your decisions are justified, then you are good! Contact the reviewers or the leads about any exceptions.

Requirements

  • Followed Coding Style Guide
  • Presented/discussed in some capacity with others on the Controls team
  • Code Build checks pass
  • No merge conflicts
  • Software feature has documentation for it updated in both ReadTheDocs and the comments
  • Software feature has documentation for it updated
  • Testing
    • Software feature has test associated with it
    • Test provides useful information and uses relevant data to accurately represent Controls
    • Tested on hardware
    • NOTE: If test file already exists, use that one
  • If applicable, have you added the new feature to main.c?
  • Tagged appropriate issue ticket on this PR
  • Did you have fun?

Things to Consider

  • Even if the above are checked, is this the best way of writing my code?
    • It's possible to write code that works, but are there ways to make code more efficient?

@Cam0Cow
Copy link
Contributor Author

Cam0Cow commented Sep 30, 2023

Not done yet, still need to remove fault state when #312 gets merged in, and then error_loc_t needs to be removed whenever #333 gets done.
Closes #357

@Cam0Cow Cam0Cow marked this pull request as ready for review November 4, 2023 16:08
Copy link
Contributor

@diyarajon diyarajon left a comment

Choose a reason for hiding this comment

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

IGN_CONT_PERIOD on line 21 is not being used so I believe that it can be deleted.

Apps/Src/main.c Show resolved Hide resolved
Apps/Src/main.c Outdated Show resolved Hide resolved
diyarajon
diyarajon previously approved these changes Nov 10, 2023
Copy link
Contributor

@NathanielDelgado NathanielDelgado left a comment

Choose a reason for hiding this comment

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

Seems fine. Just don't know if we want CommandLine as default in main.c

@Cam0Cow Cam0Cow linked an issue Nov 11, 2023 that may be closed by this pull request
@IshDeshpa IshDeshpa dismissed stale reviews from NathanielDelgado and diyarajon via 31e2d81 November 25, 2023 16:47
Cam0Cow and others added 10 commits November 25, 2023 11:41
commit f72917b
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 15:55:26 2023 +0000

    Fixed accidental deletion in docs.

commit 2755a69
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 15:50:32 2023 +0000

    Memset the gTxMessage.Data to zero since one-byte messages weren't clearing out old data in the rest of the bytes, updated comments in the test file.

commit 5d3ddfb
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 14:50:01 2023 +0000

    Added commas to enum to match new enum generation format.

commit 347ef98
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 14:27:58 2023 +0000

    Updated NUM_CAN_IDS to MAX_CAN_ID in the CANId enum.

commit 03f262c
Merge: 362628e e6c9fb6
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 10:31:31 2023 +0000

    Merge branch 'master' into sendcarcan-telemetry-merge

commit 362628e
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 10:24:37 2023 +0000

    Revisions to SendCarCAN and test file to allow messages to be inspected in GDB using variables instead of prints which were overwhelming UART.

commit 8bdf6bb
Author: Madeleine Lee <[email protected]>
Date:   Mon Nov 13 23:04:14 2023 +0000

    Addressing review comments: updated SendTritium macros, removed motorMsgCount queue, downsized SendCarCAN FIFO.

commit 0079ce4
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 11 17:57:19 2023 +0000

    Test file changes for last minute panic debugging. Added an array to see what IDS are being received.

commit de12ac1
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 11 17:32:41 2023 +0000

    Added while loop to putIOState

commit ff4bf0a
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 11 08:16:23 2023 +0000

    Updates from hardware testing: added counter array to reduce frequency at which we forwared motor messages onto CarCAN, moved init functions in test file, added Renode independent watchdog fix.

commit 77dbc2a
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 11 03:22:40 2023 +0000

    Changed PutIOState to send directly instead of putting messages in the queue to be sent by SendCarCAN later.

commit ce1d74a
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 11 03:20:33 2023 +0000

    Moved mutex, semaphore creation and fifo renewal to a new SendCarCAN_Init function so that other tasks can put things into the queue before SendCarCAN is ready. Additionally, created a new static task in SendCarCAN.c called PutIOState which periodically sends messages over CarCAN.

commit 9043b7b
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 10 23:57:18 2023 +0000

    Deleted extra lines added when merging.

commit a5e63df
Merge: d303e9b 5ae3439
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 10 23:41:44 2023 +0000

    Merge branch 'master' into sendcarcan-telemetry-merge

commit d303e9b
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 10 23:07:18 2023 +0000

    Test file updates from testing on hardware.

commit 9c755ba
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 4 21:08:49 2023 +0000

    Added ability to print FIFO queue indexes, added ReadTritium task in SendCarCAN test, fixed CAN initialization to read CarCAN but also initialize MOTORCAN.

commit 489a494
Author: Madeleine Lee <[email protected]>
Date:   Fri Oct 27 14:44:45 2023 +0000

    Cleaned up and worked on test file, removed attempt to put fifo in header file and added a wrapper instead. Test compiles but hasn't been verified to work successfully yet.

commit c38a3d7
Author: Madeleine Lee <[email protected]>
Date:   Sat Oct 21 04:57:18 2023 +0000

    SendCarCAN test file compiles! But at what cost...

commit ad84abf
Author: Madeleine Lee <[email protected]>
Date:   Sat Oct 21 04:47:33 2023 +0000

    Made aa first draft of the SendCarCAN test file and attempted to get at the SendCarCAN file by exposing it with a macro mess. controls-leader code compiles, but the test file does not compile yet.

commit 3d1d906
Author: Madeleine Lee <[email protected]>
Date:   Fri Oct 20 21:06:52 2023 +0000

    Altered test macros so that we can define __TEST_SENDTRITIUM and print info while running tests on hardware or also define __TEST_SENDTRITIUM_SOFTWAREONLY to bypass hardware inputs/outputs.

commit 73dddf7
Author: Madeleine Lee <[email protected]>
Date:   Fri Oct 20 03:02:13 2023 +0000

    Added line to put messages from the motor into the CarCAN queue to be sent.

commit 90cb9c6
Author: Madeleine Lee <[email protected]>
Date:   Fri Oct 20 02:50:50 2023 +0000

    Deleted most putIOStateTimer thigns, added a macro and counter in the sendCarCAN loop to send the IO message at approximately the same frequency as before (250 ms), but this time using loops, delays, and counters. It won't be as accurate, but it should still be at a relatively similar frequency.

commit 5185426
Author: Madeleine Lee <[email protected]>
Date:   Sat Oct 14 20:29:23 2023 +0000

    Changed main loop to check the queue periodically instead of pending on a semaphore. The loop will check two semaphores- one to see if it's time to put an IO State message in the queue (based on if the putIOState timer has expired or not) and one to see if there's a message in the queuethat we should send.

commit 2d7dbbf
Author: Madeleine Lee <[email protected]>
Date:   Sat Oct 7 23:21:43 2023 +0000

    Started a test file for SendCarCAN.

commit 2f45100
Merge: 6f69ba9 fd33899
Author: ishdeshpa <[email protected]>
Date:   Tue Sep 26 00:09:02 2023 +0000

    merged in master, now builds. test still needs to be written and bugfixes made. also ignition switch message needs to be added.

commit 6f69ba9
Author: ishdeshpa <[email protected]>
Date:   Mon Sep 25 23:50:29 2023 +0000

    merge conflict resolved

commit de426c5
Author: ishdeshpa <[email protected]>
Date:   Mon Jul 10 19:36:13 2023 +0000

    adapted CAN_Queue into a library file and integrated with SendCarCAN

commit cfd0b7c
Author: ishdeshpa <[email protected]>
Date:   Sun Jul 9 22:16:56 2023 +0000

    renamed telemetry to send car can

commit 771a916
Author: Nathaniel Delgado <[email protected]>
Date:   Sat Jul 8 03:13:27 2023 +0000

    Merged functionality of SendCarCAN into Telemetry and removed SendCarCAN remnants
…_ReadCarCAN, changed BPS_CONTACTOR CAN message ID to 0x102 instead of x101 (which is for BPS All Clear)
commit 6cf7996
Author: Madeleine Lee <[email protected]>
Date:   Sun Dec 3 05:59:52 2023 +0000

    Changes from integration with BPS: flipped reading of ignition pins in PutIOState to account for the inverted logic, added a new bit of information to PutIOState in byte 3, bit 2 which tells BPS if the array contactor should be turned on or not (true if IGN_1 or IGN_2 are on). This is necessary because  when the ignition is switched to motor state, the array pin is turned off (only one or the other is on at a time). Note: in the future we'd like to use the GPIO read settings to account for the inverted ignition logic so we don't have to negate every Minions_Read.

commit f72917b
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 15:55:26 2023 +0000

    Fixed accidental deletion in docs.

commit 2755a69
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 15:50:32 2023 +0000

    Memset the gTxMessage.Data to zero since one-byte messages weren't clearing out old data in the rest of the bytes, updated comments in the test file.

commit 5d3ddfb
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 14:50:01 2023 +0000

    Added commas to enum to match new enum generation format.

commit 347ef98
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 14:27:58 2023 +0000

    Updated NUM_CAN_IDS to MAX_CAN_ID in the CANId enum.

commit 03f262c
Merge: 362628e e6c9fb6
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 10:31:31 2023 +0000

    Merge branch 'master' into sendcarcan-telemetry-merge

commit 362628e
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 17 10:24:37 2023 +0000

    Revisions to SendCarCAN and test file to allow messages to be inspected in GDB using variables instead of prints which were overwhelming UART.

commit 8bdf6bb
Author: Madeleine Lee <[email protected]>
Date:   Mon Nov 13 23:04:14 2023 +0000

    Addressing review comments: updated SendTritium macros, removed motorMsgCount queue, downsized SendCarCAN FIFO.

commit 0079ce4
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 11 17:57:19 2023 +0000

    Test file changes for last minute panic debugging. Added an array to see what IDS are being received.

commit de12ac1
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 11 17:32:41 2023 +0000

    Added while loop to putIOState

commit ff4bf0a
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 11 08:16:23 2023 +0000

    Updates from hardware testing: added counter array to reduce frequency at which we forwared motor messages onto CarCAN, moved init functions in test file, added Renode independent watchdog fix.

commit 77dbc2a
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 11 03:22:40 2023 +0000

    Changed PutIOState to send directly instead of putting messages in the queue to be sent by SendCarCAN later.

commit ce1d74a
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 11 03:20:33 2023 +0000

    Moved mutex, semaphore creation and fifo renewal to a new SendCarCAN_Init function so that other tasks can put things into the queue before SendCarCAN is ready. Additionally, created a new static task in SendCarCAN.c called PutIOState which periodically sends messages over CarCAN.

commit 9043b7b
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 10 23:57:18 2023 +0000

    Deleted extra lines added when merging.

commit a5e63df
Merge: d303e9b 5ae3439
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 10 23:41:44 2023 +0000

    Merge branch 'master' into sendcarcan-telemetry-merge

commit d303e9b
Author: Madeleine Lee <[email protected]>
Date:   Fri Nov 10 23:07:18 2023 +0000

    Test file updates from testing on hardware.

commit 9c755ba
Author: Madeleine Lee <[email protected]>
Date:   Sat Nov 4 21:08:49 2023 +0000

    Added ability to print FIFO queue indexes, added ReadTritium task in SendCarCAN test, fixed CAN initialization to read CarCAN but also initialize MOTORCAN.

commit 489a494
Author: Madeleine Lee <[email protected]>
Date:   Fri Oct 27 14:44:45 2023 +0000

    Cleaned up and worked on test file, removed attempt to put fifo in header file and added a wrapper instead. Test compiles but hasn't been verified to work successfully yet.

commit c38a3d7
Author: Madeleine Lee <[email protected]>
Date:   Sat Oct 21 04:57:18 2023 +0000

    SendCarCAN test file compiles! But at what cost...

commit ad84abf
Author: Madeleine Lee <[email protected]>
Date:   Sat Oct 21 04:47:33 2023 +0000

    Made aa first draft of the SendCarCAN test file and attempted to get at the SendCarCAN file by exposing it with a macro mess. controls-leader code compiles, but the test file does not compile yet.

commit 3d1d906
Author: Madeleine Lee <[email protected]>
Date:   Fri Oct 20 21:06:52 2023 +0000

    Altered test macros so that we can define __TEST_SENDTRITIUM and print info while running tests on hardware or also define __TEST_SENDTRITIUM_SOFTWAREONLY to bypass hardware inputs/outputs.

commit 73dddf7
Author: Madeleine Lee <[email protected]>
Date:   Fri Oct 20 03:02:13 2023 +0000

    Added line to put messages from the motor into the CarCAN queue to be sent.

commit 90cb9c6
Author: Madeleine Lee <[email protected]>
Date:   Fri Oct 20 02:50:50 2023 +0000

    Deleted most putIOStateTimer thigns, added a macro and counter in the sendCarCAN loop to send the IO message at approximately the same frequency as before (250 ms), but this time using loops, delays, and counters. It won't be as accurate, but it should still be at a relatively similar frequency.

commit 5185426
Author: Madeleine Lee <[email protected]>
Date:   Sat Oct 14 20:29:23 2023 +0000

    Changed main loop to check the queue periodically instead of pending on a semaphore. The loop will check two semaphores- one to see if it's time to put an IO State message in the queue (based on if the putIOState timer has expired or not) and one to see if there's a message in the queuethat we should send.

commit 2d7dbbf
Author: Madeleine Lee <[email protected]>
Date:   Sat Oct 7 23:21:43 2023 +0000

    Started a test file for SendCarCAN.

commit 2f45100
Merge: 6f69ba9 fd33899
Author: ishdeshpa <[email protected]>
Date:   Tue Sep 26 00:09:02 2023 +0000

    merged in master, now builds. test still needs to be written and bugfixes made. also ignition switch message needs to be added.

commit 6f69ba9
Author: ishdeshpa <[email protected]>
Date:   Mon Sep 25 23:50:29 2023 +0000

    merge conflict resolved

commit de426c5
Author: ishdeshpa <[email protected]>
Date:   Mon Jul 10 19:36:13 2023 +0000

    adapted CAN_Queue into a library file and integrated with SendCarCAN

commit cfd0b7c
Author: ishdeshpa <[email protected]>
Date:   Sun Jul 9 22:16:56 2023 +0000

    renamed telemetry to send car can

commit 771a916
Author: Nathaniel Delgado <[email protected]>
Date:   Sat Jul 8 03:13:27 2023 +0000

    Merged functionality of SendCarCAN into Telemetry and removed SendCarCAN remnants
commit 66958b8
Author: Nathaniel Delgado <[email protected]>
Date:   Sat Dec 16 18:29:35 2023 -0600

    SendCarCAN Restructure (#337)

    * Merged functionality of SendCarCAN into Telemetry and removed SendCarCAN remnants

    * renamed telemetry to send car can

    * adapted CAN_Queue into a library file and integrated with SendCarCAN

    * merge conflict resolved

    * Started a test file for SendCarCAN.

    * Changed main loop to check the queue periodically instead of pending on a semaphore. The loop will check two semaphores- one to see if it's time to put an IO State message in the queue (based on if the putIOState timer has expired or not) and one to see if there's a message in the queuethat we should send.

    * Deleted most putIOStateTimer thigns, added a macro and counter in the sendCarCAN loop to send the IO message at approximately the same frequency as before (250 ms), but this time using loops, delays, and counters. It won't be as accurate, but it should still be at a relatively similar frequency.

    * Added line to put messages from the motor into the CarCAN queue to be sent.

    * Altered test macros so that we can define __TEST_SENDTRITIUM and print info while running tests on hardware or also define __TEST_SENDTRITIUM_SOFTWAREONLY to bypass hardware inputs/outputs.

    * Made aa first draft of the SendCarCAN test file and attempted to get at the SendCarCAN file by exposing it with a macro mess. controls-leader code compiles, but the test file does not compile yet.

    * SendCarCAN test file compiles! But at what cost...

    * Cleaned up and worked on test file, removed attempt to put fifo in header file and added a wrapper instead. Test compiles but hasn't been verified to work successfully yet.

    * Added ability to print FIFO queue indexes, added ReadTritium task in SendCarCAN test, fixed CAN initialization to read CarCAN but also initialize MOTORCAN.

    * Test file updates from testing on hardware.

    * Deleted extra lines added when merging.

    * Moved mutex, semaphore creation and fifo renewal to a new SendCarCAN_Init function so that other tasks can put things into the queue before SendCarCAN is ready. Additionally, created a new static task in SendCarCAN.c called PutIOState which periodically sends messages over CarCAN.

    * Changed PutIOState to send directly instead of putting messages in the queue to be sent by SendCarCAN later.

    * Updates from hardware testing: added counter array to reduce frequency at which we forwared motor messages onto CarCAN, moved init functions in test file, added Renode independent watchdog fix.

    * Added while loop to putIOState

    * Test file changes for last minute panic debugging. Added an array to see what IDS are being received.

    * Addressing review comments: updated SendTritium macros, removed motorMsgCount queue, downsized SendCarCAN FIFO.

    * Revisions to SendCarCAN and test file to allow messages to be inspected in GDB using variables instead of prints which were overwhelming UART.

    * Updated NUM_CAN_IDS to MAX_CAN_ID in the CANId enum.

    * Added commas to enum to match new enum generation format.

    * Memset the gTxMessage.Data to zero since one-byte messages weren't clearing out old data in the rest of the bytes, updated comments in the test file.

    * Fixed accidental deletion in docs.

    * Changes from integration with BPS: flipped reading of ignition pins in PutIOState to account for the inverted logic, added a new bit of information to PutIOState in byte 3, bit 2 which tells BPS if the array contactor should be turned on or not (true if IGN_1 or IGN_2 are on). This is necessary because  when the ignition is switched to motor state, the array pin is turned off (only one or the other is on at a time). Note: in the future we'd like to use the GPIO read settings to account for the inverted ignition logic so we don't have to negate every Minions_Read.

    * Addressed review comments: Shifted task priorities to be next to each other, added to the comment for SendCarCAN_Put.

    * addressed review comments

    ---------

    Co-authored-by: ishdeshpa <[email protected]>
    Co-authored-by: Madeleine Lee <[email protected]>

commit 5c13022
Author: Nathaniel Delgado <[email protected]>
Date:   Wed Dec 13 14:59:37 2023 -0600

    Added bps can sim for integration simulation
IshDeshpa
IshDeshpa previously approved these changes Dec 19, 2023
Copy link
Contributor

@IshDeshpa IshDeshpa left a comment

Choose a reason for hiding this comment

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

LGTM

Tests/Test_TrailingZeros.c Outdated Show resolved Hide resolved
Drivers/Src/CANbus.c Show resolved Hide resolved
Drivers/Inc/CANbus.h Show resolved Hide resolved
Drivers/Inc/CANConfig.h Show resolved Hide resolved
BSP/STM32F413/Makefile Show resolved Hide resolved
Apps/Inc/fifo.h Outdated Show resolved Hide resolved
Apps/Src/ReadTritium.c Show resolved Hide resolved
Apps/Src/SendCarCAN.c Outdated Show resolved Hide resolved
Apps/Src/SendTritium.c Show resolved Hide resolved
Apps/Src/Tasks.c Show resolved Hide resolved
Copy link
Member

@babusid babusid left a comment

Choose a reason for hiding this comment

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

Mostly looks good, but the queue saturation fix in SCC is kind of a patch job, and while it practically likely is stable, theoretically the messages it skips is completely arbitrary. I would investigate an approach with stronger guarantees.

Copy link
Contributor

@diyarajon diyarajon left a comment

Choose a reason for hiding this comment

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

Just a couple of comments and questions on the code

Apps/Src/ReadTritium.c Show resolved Hide resolved
Apps/Src/SendCarCAN.c Show resolved Hide resolved
Apps/Src/SendCarCAN.c Show resolved Hide resolved
Apps/Src/SendCarCAN.c Show resolved Hide resolved
Apps/Src/SendTritium.c Show resolved Hide resolved
Copy link
Contributor

@diyarajon diyarajon left a comment

Choose a reason for hiding this comment

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

Thanks for explaining the code from my questions last time. I left some suggestions on comments and can filter list. Very very cool cod🐟

Apps/Inc/SendTritium.h Outdated Show resolved Hide resolved
Apps/Src/SendCarCAN.c Outdated Show resolved Hide resolved
Apps/Src/SendCarCAN.c Outdated Show resolved Hide resolved
Apps/Src/SendCarCAN.c Outdated Show resolved Hide resolved
Apps/Src/main.c Show resolved Hide resolved
Drivers/Inc/CANConfig.h Outdated Show resolved Hide resolved
Copy link
Contributor

@KnockbackNemo KnockbackNemo left a comment

Choose a reason for hiding this comment

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

I left a few questions, but otherwise it looks pretty good!
I also wanted to note that since we're leaving some comments to be fixed in other issue tickets (#387, #374 , #386, #388 ), we probably want to make sure the important ones are addressed for the car (or integration/when needed).

Apps/Src/ReadTritium.c Outdated Show resolved Hide resolved
Apps/Src/ReadTritium.c Outdated Show resolved Hide resolved
Apps/Src/SendTritium.c Show resolved Hide resolved
Apps/Src/SendTritium.c Show resolved Hide resolved
Apps/Src/Tasks.c Outdated Show resolved Hide resolved
@IshDeshpa
Copy link
Contributor

Gonna start addressing these changes.

IshDeshpa and others added 4 commits December 31, 2023 18:39
Co-authored-by: Diya Rajon <[email protected]>
Co-authored-by: Madeleine Lee <[email protected]>
* Changed github actions workflow to use install script from Embedded-Sharepoint

* initialize submodules before install tools

* fixed directory

* changed flags

* changed the target that is the phony so that later we could add additional targets and change what 'leader' points to
KnockbackNemo
KnockbackNemo previously approved these changes Jan 2, 2024
Copy link
Contributor

@KnockbackNemo KnockbackNemo left a comment

Choose a reason for hiding this comment

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

I left a suggestion, but it's more related to preference and doesn't need to be followed if you disagree. Otherwise, looks good to me!

Apps/Src/SendTritium.c Show resolved Hide resolved
Apps/Src/SendTritium.c Show resolved Hide resolved
Apps/Src/SendCarCAN.c Outdated Show resolved Hide resolved
Apps/Src/SendCarCAN.c Outdated Show resolved Hide resolved
diyarajon
diyarajon previously approved these changes Jan 2, 2024
Copy link
Contributor

@diyarajon diyarajon left a comment

Choose a reason for hiding this comment

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

Looks good! I left some questions.

Apps/Src/SendCarCAN.c Show resolved Hide resolved
Apps/Src/main.c Show resolved Hide resolved
Drivers/Src/CANConfig.c Show resolved Hide resolved
Copy link
Contributor

@NathanielDelgado NathanielDelgado left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@KnockbackNemo KnockbackNemo left a comment

Choose a reason for hiding this comment

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

Looks good!

@IshDeshpa IshDeshpa merged commit 70b3526 into master Jan 2, 2024
2 checks passed
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.

Seemingly Incorrect code in SendTritium. Main.C Initialization Procedure
6 participants