-
Notifications
You must be signed in to change notification settings - Fork 3
Feat: add osm tools #17
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
Conversation
janepie
commented
Apr 15, 2025
- routing for car, bike, foot
- generating map links that make use of the OSM reference widget
| Retrieve a URL for a map of a given location. | ||
| :param profile: the kind of transport used to travel the route. Available are 'routed-bike', 'routed-foot', 'routed-car' | ||
| :param coordinates: the coordinates to route between, format is '53.5667,9.53333;53.550341,10.000654'' | ||
| :return: URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is copy pasta from above I think :)
ex_app/lib/all_tools/external.py
Outdated
| """ | ||
| Retrieve a route between two coordinates traveled by foot, car or bike | ||
| :param profile: the kind of transport used to travel the route. Available are 'routed-bike', 'routed-foot', 'routed-car' | ||
| :param coordinates: the coordinates to route between, format is '53.5667,9.53333;53.550341,10.000654'' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to split this into two arguments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm that was me being lazy and it seems to work, splitting is fine by me if there is some benefit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the benefit is to make it easier for the LLM to pass the parameters. I would be in favor of doing it just like in get_public_transport_route_for_coordinates with origin_lat, origin_lon, destination_lat and destination_lon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
The route URL should be like https://routing.openstreetmap.de/?loc=43.61333251425984%2C3.856128324437151&loc=43.622426736145854%2C3.861320548569239&srv=0
It currently is like https://routing.openstreetmap.de/routed-foot/route/v1/driving/43.7301,3.8787;43.6813,3.8305?overview=false&steps=true which is the endpoint to get the JSON, it's not the web interface to view the route on a map.
The integration_openstreetmap app provides a reference widget for the first type of link I mentioned, not for the second. -
The links are not rendered even if they are supported by a reference provider. I think this is because they are markdown links and not just the plain URL. Maybe there is something to adjust in the chat UI. The Text app renders widgets correctly for markdown links which text is different than the URL.
ex_app/lib/all_tools/external.py
Outdated
| """ | ||
| Retrieve a route between two coordinates traveled by foot, car or bike | ||
| :param profile: the kind of transport used to travel the route. Available are 'routed-bike', 'routed-foot', 'routed-car' | ||
| :param coordinates: the coordinates to route between, format is '53.5667,9.53333;53.550341,10.000654'' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the benefit is to make it easier for the LLM to pass the parameters. I would be in favor of doing it just like in get_public_transport_route_for_coordinates with origin_lat, origin_lon, destination_lat and destination_lon.
ex_app/lib/all_tools/external.py
Outdated
| Retrieve a URL for a map of a given location. | ||
| :param profile: the kind of transport used to travel the route. Available are 'routed-bike', 'routed-foot', 'routed-car' | ||
| :param coordinates: the coordinates to route between, format is '53.5667,9.53333;53.550341,10.000654'' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Retrieve a URL for a map of a given location. | |
| :param profile: the kind of transport used to travel the route. Available are 'routed-bike', 'routed-foot', 'routed-car' | |
| :param coordinates: the coordinates to route between, format is '53.5667,9.53333;53.550341,10.000654'' | |
| Get a link to a map centered on a given location. | |
| :param location: the location name or address |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I had a look again and it seems like the routing tool is not really working, it's calling the routing tool but not using the information in the answer. It answers with a route but that seems to come from general training
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Found it, lat and lon mixed up :D I guess this is a really good reason to have the coordinates in 4 parameters |
ex_app/lib/all_tools/external.py
Outdated
| """ | ||
|
|
||
| url = f'https://routing.openstreetmap.de/{profile}/route/v1/driving/{origin_lon},{origin_lat};{destination_lon},{destination_lat}?overview=false&steps=true' | ||
| map_url = f' https://routing.openstreetmap.de/?loc={origin_lat}%2C{origin_lon}&loc={destination_lat}%2C{destination_lon}&srv=0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, to prevent to tool to crash if an invalid profile param is given, it could fallback to "routed-car".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks!
Signed-off-by: Jana Peper <[email protected]>
bb8decc to
2bdf5bd
Compare
Signed-off-by: Jana Peper <[email protected]>
|
@julien-nc Link should be correct now (by foot as fallback, let's have less cars), but the widget shows the car route |
julien-nc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
but the widget shows the car route
Fine on my side with a markdown link

I'm pretty sure it's because in your context, there's a plain link followed by a dot in the response message. This messes with the automatic link resolving mechanism of NcRichText. The dot is included in the link so the value of srv becomes 2. instead of 2. This is fixed in nextcloud/assistant#221
|
Yes, works fine now! |


