-
-
Notifications
You must be signed in to change notification settings - Fork 23
RSM IOC/HW #1807
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
base: master
Are you sure you want to change the base?
RSM IOC/HW #1807
Conversation
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.
please remove the stuff added here
| file(GLOB HOMEBREW_GCC_CANDIDATES | ||
| "/opt/homebrew/bin/gcc-[0-9]*" | ||
| "/usr/local/bin/gcc-[0-9]*" | ||
| "/usr/bin/gcc" |
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 is this added here?
firmware/hexray/RSM/CMakeLists.txt
Outdated
| list(APPEND IO_SRCS) | ||
| set(IO_INCLUDE_DIRS | ||
| # "${CMAKE_CURRENT_SOURCE_DIR}/src/io" | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/src/io/" |
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.
NIT remove the / at the end?
| #include "app_canTx.h" | ||
| } | ||
|
|
||
| #define MIN_BRAKE_PRESSURE_PSI 0.0f |
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.
so you can use constexpr terms here isntead of define
|
|
||
| #include "io_coolant.hpp" | ||
|
|
||
| #define MIN_FLOW_RATE_L_PER_MIN 1.0f |
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.
same thing here please with constexpr
| #include "app_canTx.h" | ||
| } | ||
|
|
||
| #define MIN_TIRE_TEMPERATURE_CELSIUS -20.0f |
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.
samething here please
|
|
||
| } // namespace hw::adcs | ||
|
|
||
| extern "C" |
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.
there is a preprocessor define that is called CFUNC which you can use
| } // namespace hw::adcs | ||
|
|
||
| extern "C" | ||
| { |
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.
came thing here
| PwmInput flow_meter_config(&htim1, | ||
| HAL_TIM_ACTIVE_CHANNEL_1, | ||
| TIM1_FREQUENCY, | ||
| TIM_CHANNEL_1, |
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.
NIT: line it up please
| TIM1_ARR | ||
| ); | ||
|
|
||
| void HAL_TIM_IC_CaptureCallback(TIM_HandleTypeDef *htim) |
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 believe this needs to be outside of the name space and also externed C given that this is a c function
| #include "hw_adc.hpp" | ||
| #endif | ||
|
|
||
| #define BRAKE_PRESSURE_OC_THRESHOLD_V 0.33f // Change if under-voltage threshold is diff |
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.
please constexpr these guys as well
| return 0; | ||
| } | ||
|
|
||
| // bool OCSC() |
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.
is there a reason these guys are commented out?
| // namespace io::brakeLight | ||
| // { | ||
| // void set(const bool val) | ||
| // { |
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.
is there a reason this is commented out
|
|
||
| void checkIfFlowMeterActive() | ||
| { | ||
| // hw_pwmInputFreqOnly_checkIfPwmIsActive(&flow_meter_5v5); |
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.
commented out?
Changelist
Testing Done
Resolved Tickets
FIRM-553