WaterlooRobotics / mobilerobotics

Matlab and Robot code for MTE 544: Autonomous Mobile Robotics at the University of Waterloo
66 stars 38 forks source link

I11 ekf localization #49

Closed ghost closed 7 years ago

ghost commented 8 years ago

Changes Include

New Functions

LordBismaya commented 8 years ago
  1. Overall nice job done, well formatted and documented. Very clear and intuitive.
  2. I will borrow your idea of including "Instructions" as a part of my comment. But, I must add that it is advisable to remove the whole edit history and be concise about what is being done by the code.
  3. As a part of this assignment, I modified the fastSLAM code and I find many similar functions such as: control_inputs,meas_model,motion_model,linearized_meas_model,linearized_motion_model, inview/get2dpointmeasurement. Currently, we both use these entities as a part of our main program.It would be great if we separate these out and share these functions. That way, additions to these functions can be done separately without affecting our main functions ( _EKF in your case and _fastSLAM in mine). I suspect, EKF SLAM could also use these functions.
  4. I notice that feature_initialization is currently static. An improvement could be to make this random where the user just sets the no. of features and random feature allocation is done on the map. This is implemented neatly on the fastSLAM code. Do have a look. Again this can be pulled out as a separate function and can be shared.
  5. During the call of get2Dpointmeasurement variable 'p' is used. This can be renamed to a more intuitive name such as 'range/meas_distance etc'. Although, it is immediately obvious in the next step, a descriptive variable name helps. I might add that may be p can be removed and y(1:length(p),t) can be directly used instead of it. I am not entirely sure, but may be it works.
  6. Similarly, 'I and Inn' can be renamed to 'meas_error etc'.
  7. One line comments can be added on the right side of the following variables during initializations :nStates,nMeasurements,mup_S,mu_S etc to make it more informative,similar to what has been done in the get2Dpointmeasurement function.
  8. Perhaps what was intended to be done under the 'else' condition of get2Dpointmeasurement function was to use a fprintf and display it to the prompt? I realize this condition is never attained unless specifically called with an argument >3 but might as well make it entirely bug free.

The suggestions are only minor because the rest of the code is neat!.

nanwei1 commented 8 years ago