daniel-j-h / libosrmc

Pure C bindings for libosrm
MIT License
18 stars 8 forks source link

Fetch GeoJSON result from route response #10

Open srimaln91 opened 5 years ago

srimaln91 commented 5 years ago

Hi,

I want to extract the GeoJSON string from the route response. Is there a way to accomplish this?

I would be really grateful if someone can help me with this.

Thanks.

daniel-j-h commented 5 years ago

Check out the bindings for the route service

https://github.com/daniel-j-h/libosrmc/blob/38e6af24c0389854eea8065235e626b608ff8b32/libosrmc/osrmc.h#L184-L196

we are not yet providing access to the geometry. You can add it by

Let me know if you need more details on how to do this specifically.

srimaln91 commented 5 years ago

Thank you for the quick response. I am not very familiar with C++. Anyway will try to do this.

srimaln91 commented 5 years ago

Look into the below function I wrote, This works for polyline, polyline6 geometry types. When I try to fetch GeoJSON, I'm getting a segmentation violation error.

char* osrmc_route_response_geom(osrmc_route_response_t response, osrmc_error_t* error) try {
  auto* response_typed = reinterpret_cast<osrm::json::Object*>(response);

  auto& routes = response_typed->values["routes"].get<osrm::json::Array>();
  auto& route = routes.values.at(0).get<osrm::json::Object>();

  std::string geom = route.values["geometry"].get<osrm::json::String>().value;
  return (char*)geom.c_str();

} catch (const std::exception& e) {
  osrmc_error_from_exception(e, error);
  return NULL;
}

Can you help me to fetch GeoJSON string from route response when the geometry type is geoJSON.

daniel-j-h commented 5 years ago
  std::string geom = route.values["geometry"].get<osrm::json::String>().value;
  return (char*)geom.c_str();

You return a dangling pointer to a std::string's underlying data here. The std::string's destructor will get called at the end of the function leaving your pointer that you return dangling. Then accessing it results in a memory violation.

Instead it'd be best to

srimaln91 commented 5 years ago

Thank you for the kindful advices Daniel. I have allocated memory from the heap and returning the address of it. The caller function should be responsible for clearing that heap allocation. The below solution is working for polyline and polyline6 geomtry types. I'm still having a segmentation fault when I set geom type to geojson. I think the issue comes in when I try to cast geojson object into string.

char* osrmc_route_response_geom(osrmc_route_response_t response, osrmc_error_t* error) try {
  auto* response_typed = reinterpret_cast<osrm::json::Object*>(response);

  auto& routes = response_typed->values["routes"].get<osrm::json::Array>();
  auto& route = routes.values.at(0).get<osrm::json::Object>();

  const auto geom = route.values["geometry"].get<osrm::json::String>().value;

  char* geom_string = (char*) malloc(sizeof(char) * geom.length() + 1);
  strcpy(geom_string, geom.c_str()); 
  return geom_string;

} catch (const std::exception& e) {
  osrmc_error_from_exception(e, error);
  return NULL;
}

Let me know if you have any idea to get rid of this segmentation fault since I'm stuck on this.

Thanks.

daniel-j-h commented 5 years ago

Are you sure the "geometry" field is actually there? Are you sending geometries=geojson and overview=full? Otherwise I think you will not get it in the response.

Then regarding your function. Please follow the interface outlined in https://github.com/daniel-j-h/libosrmc/issues/10#issuecomment-503669006: create a new opaque geojson type, return that to the user. Then create a destruct function for that geojson type. This way we hide all memory allocation / deallocation from the user in the geojson type.

Also we probably want to use std::copy_n (plus \0 terminate) and new and delete the heap memory.

srimaln91 commented 5 years ago

I have already added functions to add geom type and overview to the route params. So I get geom in the response. The above function works perfectly when we set polyline as the geometry type. It breaks when we set geometry type to geojson.

daniel-j-h commented 5 years ago

I see. Probably because the returned geojson is not a string but a variant geojson type.

For geojson you then have to walk down the geojson tree recursively and construct a string out of it on your own. See the node bindings in osrm-backend fow how to do this.