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

[Geofencing] Support for polygon area type #194

Open
ilya-smirnov-berlin opened this issue May 17, 2024 · 7 comments · May be fixed by #195
Open

[Geofencing] Support for polygon area type #194

ilya-smirnov-berlin opened this issue May 17, 2024 · 7 comments · May be fixed by #195
Labels
enhancement New feature or request

Comments

@ilya-smirnov-berlin
Copy link

ilya-smirnov-berlin commented May 17, 2024

Problem description
Additionally to circles the Geofencing API should support polygon type areas. Circle type is rarely used as geofence. Usually a geofence describes a shape of an (often human built) geographical object like factories, buildings, city and country areas.

Possible evolution
Add polygon areas. The new datatype can be based on Geojson scheme (sorted array of 3DPoint-s) but using 2D geo type Point.
Minimal number of points in the array is naturally set to 3. Maximal number can be restricted to 16 in order to avoid possible performance issues when calculating intersections between location area and geofence for triggering entering/leaving events.

The array of geo points can be non-closed (first and last point are meant to be connected by last edge and should not be the same point) and required to be sorted. In this case the implementation should validate the polygon for edge intersections.

    AreaType:
      type: string
      description: |
        Type of this area.
        CIRCLE - The area is defined as a circle.
        POLYGON - The area is defined as polygon.
      enum:
        - CIRCLE
        - POLYGON

...

    Polygon:
      description: Polygon based area defined by an ordered geo points array
      type: object
      allOf:
        - $ref: "#/components/schemas/Area"
        - type: object
          properties:
            coordinates:
              type: array
              items:
                type: array
                items:
                  $ref: "#/components/schemas/Point"
                minItems: 3
                maxItems: 16
      example:
        areaType: POLYGON
        coordinates:
          - latitude: 52.516770
            longitude: 13.378156
          - latitude: 52.516841
            longitude: 13.379433
          - latitude: 52.515917
            longitude: 13.37959
          - latitude: 52.515849
            longitude: 13.378308
@ilya-smirnov-berlin ilya-smirnov-berlin added the enhancement New feature or request label May 17, 2024
@ilya-smirnov-berlin ilya-smirnov-berlin linked a pull request May 17, 2024 that will close this issue
@jlurien
Copy link
Collaborator

jlurien commented May 21, 2024

As discussed during the meeting, some considerations:

  • Use case is interesting but we have to take into account the complications to implement this. Circle area can be limited in the schema with radius limits, but with polygons that is not possible. Also concatenating up to 15 points may result in very complex figures, lines crossings, etc. We would need to define errors to control this.
  • Also regarding implementation, if API allows 2 possible types of geometries as inputs, we have to decide if we make mandatory for implementations to support both, or at least one, or circle as mandatory but polygon as optional... and in case of allowing partial support decide how to control this, with errors when creating subscriptions?
  • Polygon schema, in case that we proceed with this issue, should be the same as the one used in location-retrieval.
  • Does it make sense to allow polygons also for location-verification or is a separate use case?

@bigludo7
Copy link
Collaborator

Sorry to be late on this one but from my perspective the second bullet point mentioned by @jlurien is critical & must be solved before to move forward with the PR.
For the sake of the developers I prefer the 'all or nothing' approach at leas tat endpoint level but this one put a big weight on implementation. Perhaps a topic to be discussed in a larger audience.

@alpaycetin74
Copy link
Collaborator

I would allow implementers only to implement one of the available area types. If an unsupported area type is requested by the client, the implementer could return 501 or 422.

@jlurien
Copy link
Collaborator

jlurien commented May 28, 2024

Even if a implementation supports a polygon, it will likely not support any kind of polygon, e.g. a polygon as big as Europe, Moreover, there are already error scenarios with Circles that we have not detailed in the spec, e.g. if the circle is in the ocean or in another country, which is the expected behaviour when requesting a subscription? This has been mentioned in #133

So in general, we have to define the behaviour when the geofence area is not acceptable for the implementation, and there may be many reason for that: geometry not supported (if we allow polygons), area too big or small, location not supported (out of the network coverage, lack of precision), etc.

@ilya-smirnov-berlin
Copy link
Author

ilya-smirnov-berlin commented May 30, 2024

  • Use case is interesting but we have to take into account the complications to implement this. Circle area can be limited in the schema with radius limits, but with polygons that is not possible. Also concatenating up to 15 points may result in very complex figures, lines crossings, etc. We would need to define errors to control this.
  • Even if a implementation supports a polygon, it will likely not support any kind of polygon, e.g. a polygon as big as Europe, Moreover, there are already error scenarios with Circles that we have not detailed in the spec, e.g. if the circle is in the ocean or in another country, which is the expected behaviour when requesting a subscription? This has been mentioned in Define guidelines for geofencing implementation #133

We are going to implement the validation of the max size of the polygon by retrieving its two furthest points maybe additionally by calculation the area of the polygon (distance check for points will not allow thin long stripes as area). We cannot make it a part of specification like the max radius for circle areas but at least as comment in specification

  • Also regarding implementation, if API allows 2 possible types of geometries as inputs, we have to decide if we make mandatory for implementations to support both, or at least one, or circle as mandatory but polygon as optional... and in case of allowing partial support decide how to control this, with errors when creating subscriptions?

Would be a usage of http error 405 or 422 a proper case for it?

  • Polygon schema, in case that we proceed with this issue, should be the same as the one used in location-retrieval.

Done in the current pull request

  • Does it make sense to allow polygons also for location-verification or is a separate use case?

For me it's a separate use case

@alpaycetin74
Copy link
Collaborator

  • Also regarding implementation, if API allows 2 possible types of geometries as inputs, we have to decide if we make mandatory for implementations to support both, or at least one, or circle as mandatory but polygon as optional... and in case of allowing partial support decide how to control this, with errors when creating subscriptions?

Would be a usage of http error 405 or 422 a proper case for it?

405 is more appropriate for cases where the combination of the api URI and the http method is not defined in the api spec.

@jlurien
Copy link
Collaborator

jlurien commented Jun 3, 2024

422 is the error code that is being established for cases where the input values are not acceptable, due to server-side logic. It could serve as response for inputs which are syntactically correct but semantically not valid.

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

Successfully merging a pull request may close this issue.

4 participants