VFH Decision Module #54
Conversation
Mmoyv27
changed the title
| :param wide_valley_threshold: Number of sectors distinguishing wide valleys from narrow ones. | ||
| """ | ||
| self.density_threshold = density_threshold | ||
| self.min_consec_sectors = min_consec_sectors |
There was a problem hiding this comment.
following WARG style guide, this variable name should not be short-form. it can be renamed to min_consecutive_sectors. (https://uwarg-docs.atlassian.net/wiki/spaces/CV/pages/2253226033/Python+Style+Convention#Naming)
There was a problem hiding this comment.
got it, will just do
|
|
||
| :param density_threshold: The threshold for identifying candidate valleys. | ||
| :param min_consec_sectors: Minimum number of consecutive sectors for a valid valley. | ||
| :param wide_valley_threshold: Number of sectors distinguishing wide valleys from narrow ones. |
There was a problem hiding this comment.
why do we define a wide valley threshold? is a narrow valley not wide enough? is this the minimum narrow valleys required for the drone to fly down the valley? if so, can the description make this more clear?
There was a problem hiding this comment.
it's for a refinement outlined in the research paper. Technically we can go lower than the wide_threshold, but that would require us to slow down our flight_speed (formula given in paper, relates to maximal steering speed) so could implement that in the future.
There was a problem hiding this comment.
(also its just a bit safer, until we concretely decide on a safe threshold based on tests)
modules/vfh_decision/vfh_decision.py
Outdated
| far_angle = valley[-1].angle_end | ||
|
|
||
| # Ensure the valley contains 0° and has enough space on both sides | ||
| if ( |
There was a problem hiding this comment.
why does our valley at 0 degrees have to be a wide valley?
There was a problem hiding this comment.
i made an oversight. our candidate valley search should also rely on wide-valley threshold. so candidate valleys in the pre-check and in the general search should be > wide_valley threshold, not min_consec_sectors
Initial commit for vfh_decision.
followed shortly by integration tests