FIRM-167 Hexray SoC - #1976
Conversation
Lucien950
left a comment
Lucien950
left a comment
There was a problem hiding this comment.
i don't think this is a full implementation, because i don't think the unhappy path is not well covered. please make sure that is implemented and tested
| std::expected<void, FileSystemError> requireStorageReady() | ||
| { | ||
| if (!soc_storage_ready) | ||
| { | ||
| return std::unexpected(soc_storage_status); | ||
| } | ||
|
|
||
| return {}; | ||
| } |
There was a problem hiding this comment.
for example here if the storage is not ready you don't need to know why it's not ready, just that it isn't
also this is highkey just a sanity check, and should not be part of your hot path
and also the error should probably be handled where it is thrown so the type of the error is not of interest to the consumer of this function
| if (auto result = recordStatus(io_fileSystem_init()); !result) | ||
| { | ||
| LOG_ERROR("SoC storage unavailable: failed to initialize file system"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
i would decouple this functionality from soc and put it into tasks, because this is a global property
There was a problem hiding this comment.
you should still check if this fails (which is kept track of with io_fileSystem) before continuing
There was a problem hiding this comment.
wait, the checkmount on the filesystem class is private tho how can I see feedback of the init?
| constexpr uint32_t SOC_TENTHS_UNAVAILABLE = UINT32_MAX; | ||
|
|
||
| FileSystemError soc_storage_status = FILE_MOUNT_FAILED; | ||
| bool soc_storage_ready = false; |
There was a problem hiding this comment.
i think you can implicitly store this by making soc_fd an optional type, which forces an unwrap
There was a problem hiding this comment.
on the lowest of keys what is stopping us from storing the SOC from just inside app_soc?
There was a problem hiding this comment.
cause i think this can be massively simplified and just slapped in there
|
also I swear you are exposing a whole bunch of functions you don't use, if you don't use them please don't expose them |
Co-authored-by: Edwin <20777515+Lucien950@users.noreply.github.com>
| can_rx_queue.init(); | ||
| can_tx_queue.init(); | ||
|
|
||
| app::can_tx::BMS_Hash_set(GIT_COMMIT_HASH); |
| can_rx_queue.init(); | ||
| can_tx_queue.init(); | ||
|
|
||
| app::can_tx::BMS_Hash_set(GIT_COMMIT_HASH); |
There was a problem hiding this comment.
| app::can_tx::BMS_Hash_set(GIT_COMMIT_HASH); | |
| app::can_tx::BMS_Hash_set(GIT_COMMIT_HASH); |
2 participants
Changelist
NOTE: LUT for cells and function calls for segments need to be replaced when each are done. Lastly I still gotta fix test so they build 🥀
Since the "C" version of the file system references the "C" functions for hw_sd_present(), had to call hw directly from add layer. Having the new C++ version should fix.
Testing Done
Resolved Tickets
FIRM-167
FIRM-367