Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Field Creation Wizard and Field Class Changes #183

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

LukasBaecker
Copy link
Contributor

@LukasBaecker LukasBaecker commented Sep 20, 2024

In the course of planning the new field navigation #180 , the wizard for creating fields will be updated and will be accessible from the start page (the field_planner will be deactivated temporary). In addition, fields will be saved differently from now on. Only the AB line and a few field parameters will be saved persistently. Rows and the perimeter of the field will from now on be calculated dynamically from the parameters. For now we concentrate on only one field for one robot. Also, obstacles will be added in future and are neglected.

Todos:

  • update Field Creation Wizard and move it to the main page
  • update Field class (save needed params and calculate rows and boundary)
  • deactivate field_planner
  • visualize the field in leaflet
  • fix rows and boundary calculation issue
  • update tests

@pascalzauberzeug pascalzauberzeug mentioned this pull request Sep 26, 2024
8 tasks
Copy link
Member

@rodja rodja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just had a short glance at the PR, but want to share some thoughts:

  • code which we not use anymore should be removed not left in a comment (we have git to not loose code)
  • I find it quite hard to differentiate between Field and FieldParameters; I think the code would get much simpler if we simply had a Field with first_row_start, first_row_end, row_spacing, and row_countwhich are persistent. Whenever they change we would call an internal method_refreshwhich updates the non-persistent propertiesrowsandpoints`.
  • There are still some leftovers like obstacle property in Field which we do not need (yet).

@LukasBaecker
Copy link
Contributor Author

I just had a short glance at the PR, but want to share some thoughts:

  • code which we not use anymore should be removed not left in a comment (we have git to not loose code)
  • I find it quite hard to differentiate between Field and FieldParameters; I think the code would get much simpler if we simply had a Field with first_row_start, first_row_end, row_spacing, and row_countwhich are persistent. Whenever they change we would call an internal method_refreshwhich updates the non-persistent propertiesrowsandpoints`.
  • There are still some leftovers like obstacle property in Field which we do not need (yet).

I have made changes according to your feedback.

Now there seems to be an error when creating a field. The first field is created correctly, but the following ones have errors in the calculation of the rows and the perimeter, which I need to solve.

@LukasBaecker
Copy link
Contributor Author

I just had a short glance at the PR, but want to share some thoughts:

  • code which we not use anymore should be removed not left in a comment (we have git to not loose code)
  • I find it quite hard to differentiate between Field and FieldParameters; I think the code would get much simpler if we simply had a Field with first_row_start, first_row_end, row_spacing, and row_countwhich are persistent. Whenever they change we would call an internal method_refreshwhich updates the non-persistent propertiesrowsandpoints`.
  • There are still some leftovers like obstacle property in Field which we do not need (yet).

I have made changes according to your feedback.

Now there seems to be an error when creating a field. The first field is created correctly, but the following ones have errors in the calculation of the rows and the perimeter, which I need to solve.

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants