Conversation
…sion file reading errors are captured in the logger correctly
wfsyre
left a comment
wfsyre
left a comment
There was a problem hiding this comment.
No reviews on your most recent commit because its WIP
src/log/Logger.cpp
Outdated
| Logger::~Logger() { | ||
| close(); | ||
| std::lock_guard<std::mutex> lock(mutex_); | ||
| // If init_dir was never called and we have pending messages, write to fallback |
There was a problem hiding this comment.
Can we get a warning or something here indicating the user won't find logs in the spot they were expecting (or where to find them if they never set it)
src/simcontrol/SimControl.cpp
Outdated
|
|
||
| // Setup the log directory if it is required | ||
| // Always create the log directory and redirect the text logger to it. | ||
| mp_->create_log_dir(); |
There was a problem hiding this comment.
I see why you did this, but its still a little upsetting that if a user wants no logging, the mission parser will still generate a folder on their system and then just not put logs in it. It would be nice if we could run with 0 footprint if asked to
src/parse/ParseUtils.cpp
Outdated
| return true; | ||
| } | ||
| } | ||
| LOG_WARN("Terrain '" << terrain_name << "' not found or files missing (polydata/texture)"); |
There was a problem hiding this comment.
Lets add the path that we expect terrain files to be in to this warning (SCRIMMAGE_DATA_PATH)
| pid.set_is_angle(true); | ||
| } else { | ||
| double i_lim = std::stod(str_vals[3]); | ||
| pid.set_integral_band(i_lim); |
There was a problem hiding this comment.
Lets set pid.is_angle(false) explicitly here. Not your code really but while you're here we can make this 1% more readable
| auto run = [&](auto& s) { return s->step_loop_timer(temp_dt) ? s->step() : true; }; | ||
| success = std::all_of(sensors.begin(), sensors.end(), run); | ||
| try { | ||
| if (task_type == Task::Type::AUTONOMY) { |
There was a problem hiding this comment.
This block that uses step_loop_timer is one of those things that's been incorrect for awhile. t and dt should be updated based on what was last sent to the plugin. So first loop should be t and dt/num_loops and next loop should be t + 1 * (dt / num_loops). This keeps the plugins agnostic to the fact that they are looping and instead just feeds them normal t and dt as if they were just running with a higher rate. Maybe a fix for another commit, but if you see and easy way of doing this here then I think its a good time
| << endl; | ||
| cout << "Warning: Use Asset Environment versions Linux-v1.3.1+." << endl; | ||
| LOG_INFO("image channels: " << a.img.channels()); | ||
| LOG_WARN("Warning: Old AirSim Linux Asset Environments have 4 channels. Color images will not display correctly."); |
There was a problem hiding this comment.
I think your logger already prepends the Warn so we can remove "Warning" from these ones
2 participants
Going to add some unit tests to make sure it looks good, so stay posted.