purduesigbots / pros

Source code for PROS kernel: open source C/C++ development for the VEX V5 microcontroller
https://pros.cs.purdue.edu
Other
259 stars 76 forks source link

✨Add list_files function #615 #615

Open Gracelu128 opened 11 months ago

Gracelu128 commented 11 months ago

Summary:

Adds functionality to list the files on a sd card.

Motivation:

Made wrapper function for the CPP api that returns a vector of strings of file names. Renamed original function as list_files_raw in both CPP api and C api.

Test Plan:

djava commented 11 months ago

@Gracelu128 it would be helpful if, when you make changes to address review comments I've made, you could respond to the review comment to let me know (maybe with the commit hash that you addressed it in as well). Would you be able to go through and do that here so it's easier to dig through this pile of commits?

Thanks!

djava commented 11 months ago

Looks a lot better, thanks for the changes! Most of what's left is small or stylistic.

djava commented 11 months ago

Just a thought - right now there's no standard prefix to the error messages, so there's not really a clean way to programmatically check if the function errored. Perhaps they can all start with "ERROR - " or similar, so that one can at least write something like this to check for errors?

auto file_list = pros::usd::list_files("/");
if (file_list.front().starts_with("ERROR")) {
    // do error stuff
} else {
    // We know file_list is valid now
}

I'm not sure how you'd do this as is. errno isn't reliable, since it could have been set by a previous function call, and theres no return code from this function that tells us succinctly that we have to check errno, right?

Gosh, thank god for C++20 adding std::string::starts_with. Not sure what the clean way to do this otherwise would be.

Gracelu128 commented 11 months ago

Looks a lot better, thanks for the changes! Most of what's left is small or stylistic.

I just implemented your suggested stylistic changes in the new commit 490a3ca83be775d3e2c97da582e00099efb6393a.