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

Added map functionality and additional attributes #213

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

torstenknoefel
Copy link

I have married up your repo (V 5.5.0) with the code from this repo: https://github.com/SmarthomeAddicted/Indego-Integration.
(I used diff to add the missing code and adapted slightly.)

This relates to my feature request #212

@sander1988
Copy link
Collaborator

@torstenknoefel - Thank you for your work!

At what interval are you updating the map and when? Based on the mower state?

I like the feature but I see a risk here of getting blocked by the Bosch API if we increase the API calls.

I think we can merge this, but keep the feature disabled by default. Users that need it can enable it through the integration config page.

@sander1988 sander1988 changed the base branch from master to develop May 20, 2024 12:26
@sander1988 sander1988 added enhancement New feature or request question Further information is requested labels May 24, 2024
@sander1988 sander1988 self-assigned this May 24, 2024
@sander1988
Copy link
Collaborator

Just noticed something... I don't think it will work correctly with multiple mowers. The SVG uses a static file/path. So the SVG will be overwritten with multiple mowers.

Copy link

This pull request is stale because it has been open for 14 days with no activity.

@github-actions github-actions bot added the stale Stale issues or pull requests label Jun 11, 2024
@mateusz-lichota
Copy link

Hi! I'm interested in working on this PR. What needs to be done before merging? @jm-73

@sander1988
Copy link
Collaborator

sander1988 commented Jun 15, 2024

Hi @mateusz-lichota! We have not received any replies to my questions in this PR.

To summarize what's discussed/asked:

  • My questions regarding impact on number of API requests.
  • Make this feature optional through config ; disabled by default.
  • Check the possible SVG issue with multiple mower. I have not tested, but I suspect a bug based on looking at the code.

I just released version 5.7.0 ; so also sync with latest version on develop branch.

Just let me know when all is implemented/fixed so I (and maybe others) can test it.

@github-actions github-actions bot removed the stale Stale issues or pull requests label Jun 15, 2024
@mateusz-lichota
Copy link

Regarding your concerns:

  • The number of API calls: Currently 4 requests to the API are made every 10 minutes. This PR increases that to 5, which I don't think could result in being blocked
  • Making the feature optional: Downloading the map is exposed as a HA service that has to be executed manually by the user. I'll change the code in a way which only performs any tasks related to position updates if the map has been downloaded.
  • Multiple mowers: The service for downloading the map allows you to choose any name for the file, but the code that performs position updates will only work if you use the default filename, and as you said, will fail if there are multiple mowers. I'm in favor of removing the ability to choose a custom filename (since it adds no real value), and instead add the unique mower id to the filename.

I'll copy the map-related changes (discarding the alert-related stuff, which should arguably be a separate PR) from @torstenknoefel 's branch into my fork and make those changes, then open a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants