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

Adding Features #182

Merged
merged 18 commits into from
Sep 5, 2023
Merged

Adding Features #182

merged 18 commits into from
Sep 5, 2023

Conversation

kimzeuner
Copy link
Contributor

In this Pull Request i have added some Features from an old Indego Integration that was able to download the map, delete alerts etc. Since Single Key activation this was no longer running.
With the added Service "download map" it is now possible to download the map.

Added several Entities like Map, Alert ...
Added Entities for Map, Alert ...
Added requirement svgutils and sh
Added service for donwloading map, reading and deleting alerts
@LarsLautrup
Copy link

LarsLautrup commented May 26, 2023

It works! Thanks A LOT!!! 👍

mapWithIndego

@jm-73
Copy link
Collaborator

jm-73 commented May 26, 2023

Will have a look at this tomorrow or sunday.
Nice work!

@FBruynbroeck
Copy link

Hi everyone,

If you're interested, I'm also working on a feature that allows you to get the map in dashboard from a camera entity (with a map refresh every 10 seconds).

I don't want to make a pull request for the time being because I want to test the feature over a long period and fix any bugs before proposing it.

Perhaps we could merge this pull request with my fork if you're interested...

My fork: https://github.com/FBruynbroeck/Indego/tree/issue133_garden_map

Best regards

@jm-73
Copy link
Collaborator

jm-73 commented Jun 4, 2023

Is the code adjusted so it doesnt flood the API with several calls per second/minute?

@jm-73
Copy link
Collaborator

jm-73 commented Jun 11, 2023

Is the code adjusted so it doesnt flood the API with several calls per second/minute?

??? @kimzeuner

@kimzeuner
Copy link
Contributor Author

Sorry, i thought FBruynbroeck was meant.
I can update my fork with the adjustments made by FBruynbroeck and sander1988, at the moment im on vacation so im not sure if i will be able to do this in the next 2 weeks but i will let you know when im done.

@SmarthomeAddicted
Copy link
Collaborator

Hello Guys,

I already implemented this feature in a version which I made on my own based on this integration. But in my version it doesn't make a request e.g. every 10 seconds to get the map. I have one call service where you can download the map to your home assistant and also adjust the path in that call service. And then I replace the X and Y Postion with placeholders and every time when the position of the indego changed it automatically change it in the map which is in .svg format. To be honest I'm not quite family with the gitHub thing. If someone can help me maybe we can get this feature in this integration here.

BR

@SmarthomeAddicted SmarthomeAddicted mentioned this pull request Jun 26, 2023
@kimzeuner
Copy link
Contributor Author

Hi Guys,

Sorry for my late answer but i was pretty busy in the last weeks.
I have update all Files to the latest stable release and added the map/camera feature from @FBruynbroeck and also added read and delete functions for alerts.
The workaround as mentioned in #193 is not included here.

Regards

@Pr0mises
Copy link

Pr0mises commented Aug 19, 2023

@kimzeuner thanks for your work. I think you pushed it into the wrong branch as I've seen the commits inside the master branch which is not included in this fork

@kimzeuner
Copy link
Contributor Author

You are right. I will change it on monday and let you know when it is finished.

Update for map and alert features
update for map and alert features
update for map and alert features
update for map and alert features
update for map and alert features
update for map and alert features
update for map and alert features
update for map and alert features
update for map and alert features
update for map and alert features
update for map and alert features
update for map and alert features
@kimzeuner
Copy link
Contributor Author

Good morning guys,
i have now update all files from develop branch so everything should be fine now.

@Pr0mises
Copy link

Pr0mises commented Aug 21, 2023

Thank you, I tried your latest push but it seems like something is wrong with the config flow.
I just replaced all files (delted pycache) and restarted hass and received "Invalid Migration". After that I removed Bosch Indego and the credentials, restarted HASS and wanted to add it again.
Now I receive Error Config flow could not be loaded: {"message":"Invalid handler specified"}
No other logs could be found for it.

@kimzeuner
Copy link
Contributor Author

Ok, on saturday you said that you have seen my latest changes in the master branch instead of developer branch. Have you tried the files from master branch ? Did this work ?

Another idea, did you change the line
serial_number=self._serial,
to
#serial_number=self._serial,
in the init.py as mentioned as workaround in issue #193 ? I have not implemented this in my files.

@Pr0mises
Copy link

I didn't try the latest change in your master branch, only seen you pushed them into the wrong branch and wanted to let you know about it. Also you already implemented the fix mentioned in #193

@kimzeuner
Copy link
Contributor Author

Ok strange as i just did copy and paste from my running system to the files on guthub. I will have a look on it, maybe there was a mistake.

@Pr0mises
Copy link

Pr0mises commented Aug 21, 2023

That's weird indeed. I'll check if it's possible to debug the config flow somehow.

Edit: found the problem, you used the mixins.py inside the config_flow.py changing the config_flow.py back to the default fix the issue

update after copy and paste mistake
@kimzeuner
Copy link
Contributor Author

@Pr0mises i think i have good news for you. i have compared all files from my develop branch and my running system and there was a copy and paste mistake in the config_flow.py
i justupdated the file, i think it should work now.
sorry for the inconveniences.

@jm-73 jm-73 merged commit e50e974 into sander1988:develop Sep 5, 2023
@jm-73
Copy link
Collaborator

jm-73 commented Sep 5, 2023

This PR is published in the beta 5.3.0.
Happy testing! Let me know if this works for you all!

@sytchi
Copy link

sytchi commented Sep 5, 2023

I am not able to call new services like

service: indego.delete_alert_all
data:
  alert_index: '0'

I get this error

This error originated from a custom integration.

Logger: homeassistant.helpers.script.websocket_api_script
Source: custom_components/indego/__init__.py:864
Integration: Bosch Indego Mower (documentation)
First occurred: 5:58:49 PM (1 occurrences)
Last logged: 5:58:49 PM

websocket_api script: Error executing script. Unexpected error for call_service at pos 1: Alert.__init__() got an unexpected keyword argument 'data'
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 468, in _async_step
    await getattr(self, handler)()
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 703, in _async_call_service_step
    response_data = await self._async_run_long_action(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 665, in _async_run_long_action
    return long_task.result()
           ^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/core.py", line 1974, in async_call
    response_data = await coro
                    ^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/core.py", line 2011, in _execute_service
    return await target(service_call)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/indego/__init__.py", line 404, in async_delete_alert_all
    await hass.data[DOMAIN][entry.entry_id]._update_alerts()
  File "/config/custom_components/indego/__init__.py", line 864, in _update_alerts
    await self._indego_client.update_alerts()
  File "/usr/local/lib/python3.11/site-packages/pyIndego/indego_async_client.py", line 204, in update_alerts
    self._update_alerts(await self.get("alerts"))
  File "/usr/local/lib/python3.11/site-packages/pyIndego/indego_base_client.py", line 199, in _update_alerts
    self.alerts = [Alert(**a) for a in new]
                  ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/pyIndego/indego_base_client.py", line 199, in <listcomp>
    self.alerts = [Alert(**a) for a in new]
                   ^^^^^^^^^^
TypeError: Alert.__init__() got an unexpected keyword argument 'data'

@Pr0mises
Copy link

Pr0mises commented Sep 5, 2023

@sytchi all of them? All calls are working on my side

@SmarthomeAddicted
Copy link
Collaborator

SmarthomeAddicted commented Sep 6, 2023

I am not able to call new services like

service: indego.delete_alert_all
data:
  alert_index: '0'

I get this error

This error originated from a custom integration.

Logger: homeassistant.helpers.script.websocket_api_script
Source: custom_components/indego/__init__.py:864
Integration: Bosch Indego Mower (documentation)
First occurred: 5:58:49 PM (1 occurrences)
Last logged: 5:58:49 PM

websocket_api script: Error executing script. Unexpected error for call_service at pos 1: Alert.__init__() got an unexpected keyword argument 'data'
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 468, in _async_step
    await getattr(self, handler)()
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 703, in _async_call_service_step
    response_data = await self._async_run_long_action(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 665, in _async_run_long_action
    return long_task.result()
           ^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/core.py", line 1974, in async_call
    response_data = await coro
                    ^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/core.py", line 2011, in _execute_service
    return await target(service_call)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/indego/__init__.py", line 404, in async_delete_alert_all
    await hass.data[DOMAIN][entry.entry_id]._update_alerts()
  File "/config/custom_components/indego/__init__.py", line 864, in _update_alerts
    await self._indego_client.update_alerts()
  File "/usr/local/lib/python3.11/site-packages/pyIndego/indego_async_client.py", line 204, in update_alerts
    self._update_alerts(await self.get("alerts"))
  File "/usr/local/lib/python3.11/site-packages/pyIndego/indego_base_client.py", line 199, in _update_alerts
    self.alerts = [Alert(**a) for a in new]
                  ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/pyIndego/indego_base_client.py", line 199, in <listcomp>
    self.alerts = [Alert(**a) for a in new]
                   ^^^^^^^^^^
TypeError: Alert.__init__() got an unexpected keyword argument 'data'

@sytchi did you use the" load with sample data" button in HA ? it should be the number without '' so like:

service: indego.delete_alert_all
data:
  alert_index: 0

@SmarthomeAddicted
Copy link
Collaborator

@kimzeuner I saw you find a solution for the issue that I had in my version that I created a manual camera entry where the .svg file was uploaded after every change of the position. Thanks for that! Because I’m not quite familiar with the camera.py yet how the map will be updated on your version? I saw that every time the map will be new downloaded instead of just update the position with the x and y coordinates is it correct? I’m just asking because @jm-73 mentioned that to download the map every 10 seconds e.g. would stress out the Bosch server and they could be find a way to avoid that we are hammering requested to their API .

@kimzeuner
Copy link
Contributor Author

Honestly it was not me who found this solution. i have just collected some features from your data (read and delete alerts) and the data from @FBruynbroeck. In the description of the latest version of camera.py (by FBruynbroeck can be found here) it says, that the map will be download after each return to the dock but i`m not sure how often the position is updated.

@FBruynbroeck
Copy link

Hello !

You've used "my" camera feature, but it's still being tested.
Improvements may be in order.

I think it's still too early to offer this feature to everyone.

What's more, I'm currently testing it with an older version of HASS (2023.5.4).
Then I'll have to test it with the latest version.

To answer your question, the position (x/y) is retrieved and displayed on the map every 10 seconds for preview (default value of CameraImageView rendering) and every 5 seconds for stream (frame interval) BUT only when the robot is moving.
The garden map (without the robot's position) is downloaded each time the robot returns to the station.

If the camera is not displayed, the position/map will not be retrieved.
It's the rendering of the camera widget that makes the call to the api.

We need to be very careful about the number of calls we make to the Bosch servers.
1 call every 10 seconds for 40-50 minutes (robot movement) with a break for +/-30 minutes (robot recharging) has worked for me for over 3 months.
But will it continue to work if several of us are doing the same thing?
Will it work if we leave the stream active for a long time (with a refresh time of 5 seconds)?
I haven't had enough user feedback to know...

@sytchi
Copy link

sytchi commented Sep 6, 2023

@Pr0mises

@sytchi all of them? All calls are working on my side

All of the new alert related ones.

@SmarthomeAddicted

@sytchi did you use the" load with sample data" button in HA ? it should be the number without '' so like:

I did try both with the same effect

@Pr0mises
Copy link

Pr0mises commented Sep 6, 2023

@sytchi did you install it with hacs?
Try redownloading it and maybe readding to hass

@jm-73
Copy link
Collaborator

jm-73 commented Sep 7, 2023

FBruynbroeck

This puts this bet arelease in a difficult situation. I really appreciate the efftort @kimzeuner has put into this PR, but mixing bug fixes with additional features is not best-practise.

I will have to remove this beta as @FBruynbroeck clearly says that his camera functinos still needs testing. It is not ready for the masses.

Please make a new PR with the bug fix for the serial-thingie error.

/Jens

@FBruynbroeck
Copy link

@jm-73 I agree with you.
We mustn't mix up bug fixes and new features.
Let me handle the pull request for the camera feature.
Feel free to test my fork and give me feedback, I'm very interested.

And if anyone wants to work on this feature with me, you're welcome! 👍🙂

@jm-73
Copy link
Collaborator

jm-73 commented Sep 7, 2023

New release 5.4.0 instead. Have removed everything except the bug fix with the serial.

@kimzeuner kimzeuner mentioned this pull request Sep 7, 2023
@kimzeuner
Copy link
Contributor Author

ok, i completely agree with that. sorry for beeing too fast. i have tested the camera feature for a longer time now and had no issues yet.
i just created a new PR #198 which only contains the read and delete alert functions.

@jm-73
Copy link
Collaborator

jm-73 commented Sep 7, 2023

NP.
Good, appreciated!

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.

7 participants