sil architecture for tms

[ManushPatell]

No description provided.

[ManushPatell]

2 main things I wanted to bring up:

We have a collision with the macro CRC

• STM32 Hal defines a macro called #define CRC ((CRC_TypeDef *) CRC_BASE)
• Cangen creates an accessor with the same name in veh_messages.hpp, inside of class RxCanFlashCRC called inline uint8_t CRC() const { return crc_; }

I included a #undef CRC in app.cc, to include the HAL version of CRC. This will be a problem with other ECU's during re-architecture

Secondly, since we initially used atomic_buffer.hpp in main.cc and now we ported it over to tms-common logic, we ran into a problem with LDF not finding the header file atomic buffer because its now getting compiled into a library instead. I'm not too familiar with the concept, so I'm trying to learn more. Currently, the temporary fix is telling LDF that tms-common requires atomic buffer using a json file. I'm not personally a fan of this solution since it feels like we're just patching it and how this .json will have to be made for all ECU's re-architecture as well.

[BlakeFreer]

Don't use using in a header file. It pollutes the namespace. Write out the full path

It's ok to use in in the cc file

[BlakeFreer]

What's the 50.0 factor here?

It should be a named constant

[ManushPatell]

This factor was already implemented prior. Additionally, the logic before made no sense. Take a look:

TxTMSValues TMSBroadcast(const std::array<float, kSensorCount>& temperatures) { return TxTMSValues{ .val1 = static_cast<uint8_t>(bindings::temp_sensor_adc_1.ReadVoltage() * 50.0f), .val2 = static_cast<uint8_t>(bindings::temp_sensor_adc_2.ReadVoltage() * 50.0f), .val3 = static_cast<uint8_t>(bindings::temp_sensor_adc_3.ReadVoltage() * 50.0f), .val4 = static_cast<uint8_t>(bindings::temp_sensor_adc_4.ReadVoltage() * 50.0f), .val5 = static_cast<uint8_t>(bindings::temp_sensor_adc_5.ReadVoltage() * 50.0f), .val6 = static_cast<uint8_t>(bindings::temp_sensor_adc_6.ReadVoltage() * 50.0f), }; }

We pass in temperatures but read from bindings, which doesnt make sense. This version actually uses the passed in param

[BlakeFreer]

Yeah that is pretty bad. Global variables are not good

Glad to have it fixed.

You should open a github issue "figure out what this TMS constant is"

[BlakeFreer]

Regenerate the code from cubemx and run post_cubemx.sh. These files shouldn't have a git diff. They should've just been renamed

[BlakeFreer]

Good, this is exactly how bindings should be replaced

[BlakeFreer]

Currently, the temporary fix is telling LDF that tms-common requires atomic buffer using a json file.

This is exactly what you should do. Each project needs to specify its dependencies. That's a good thing not "repetitive"

[ManushPatell]

I autogenerated from stm32cubemx, and then ran the post_cubemx.sh script. I can notice this when I look at the files on github:

Otherwise I don't notice a difference...

[BlakeFreer]

nice