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

[location-verification / geofencing]: decrease radius minimum to "1" for circle-area #285

Merged

Conversation

maxl2287
Copy link
Contributor

@maxl2287 maxl2287 commented Jan 8, 2025

What type of PR is this?

Add one of the following kinds:

  • enhancement/feature

What this PR does / why we need it:

This PR decreses the minimum of a circle-area radius to "1".
The provider of the API can restrict then the minimum and respond with HTTP-422 if the size of the area is to small and cannot be covered.

Which issue(s) this PR fixes:

Fixes #282

Special notes for reviewers:

Changelog input

 release-note
* [location-verification / geofencing]: decrease radius minimum to "1" for circle-area

@maxl2287 maxl2287 added enhancement New feature or request Spring25 labels Jan 8, 2025
@maxl2287 maxl2287 self-assigned this Jan 8, 2025
bigludo7
bigludo7 previously approved these changes Jan 8, 2025
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but hope we'll have some collision with #281 for location-verification.

Co-authored-by: Jose Luis Urien <jlurien@gmail.com>
@maxl2287
Copy link
Contributor Author

waiting for @jlurien and @javier-carrocalabor if this could be part of Spring25 Meta Release

@javier-carrocalabor
Copy link

Checked the potential privacy issues.
@jlurien it's ok for me to proceed with the change of the minimum value for radius to 1m.

bigludo7
bigludo7 previously approved these changes Jan 15, 2025
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

jlurien
jlurien previously approved these changes Jan 15, 2025
@james-emerson
Copy link

james-emerson commented Jan 20, 2025

Having reviewed this from an aggregator and customer developer perspective there are some things that we should discuss and define.
1. Developer Guidance on Input Radius

  • Changing to 1m has to come with guidance to developers about when they can use this level of precision.
  • Without knowing the network circle radius in advance of making the API call this will always result in FALSE or PARTIAL responses because the input circle will always either be fully inside or outside the network circle.
  • The PARTIAL responses in this scenario will always result in the same matchRate because the input circle is contained in the network circle.
    We need to provide guidance to developers.

2. Interpreting Error Messages
LOCATION_VERIFICATION.AREA_NOT_COVERED​
LOCATION_VERIFICATION.INVALID_AREA
There needs to be some guidance about what these error messages mean and that has to be standard across operators in order to be able to aggregate the experience across operators for our developers.
I've attached some slides to illustrate these challenges and look forward to discussing here and at the next WG
DLV Precision Explained.pdf

@javier-carrocalabor
Copy link

I totally understand your concerns, @james-emerson, and agree we should be able to provide better understanding to the users of the API. I think that removing the 2km minimum value is a big progress, as all of us, CSPs and users of the APIs, were missing use cases that were feasible. Now those use cases are at reach, but they are not easy to manage, especially thinking in cross-CSPs compatibility (network deployments may differ among CSPs, or different technical restrictions may apply to CSPs in the same zones).
I think you point to the ideal solution, which would be the CSP to provide the geographical precision in advance, but that it is not easy to manage. So, I guess that the basic indication a CSP can end up giving to the API users would be "try to know the type of the zone you are asking for (urban/suburban/rural) and use the maximum radius you can tolerate in your use case in that type of zone". In any case, if you haven't seen it yet, @james-emerson, please have a look at de doc at #85 (comment) which can be aligned with what you are demanding. It can be a starting point to complete what the API users need.
This said, I would like to know other opinions or specific proposals, and also agree that more documentation should be provided for:

  1. AREA_NOT_COVERED, based on [Verification/Geofencing]: Decrease the minimum of a Circle radius #282 (comment) and example of the applicable text could be, "Legal restrictions regarding privacy or other regulatory issues may force the CSP to set a minimun value to the requested radius. In these cases, this error will be returned and the error message will refer to the reason of the limitation".
  2. INVALID_AREA: I haven't managed to find references to this error in this group. I think this one is more related to Geofencing or Population Density APIs, where, for example, malformed polygons can be bound in the requests (https://github.com/search?q=org%3Acamaraproject%20invalid_area&type=code). I don't know if this error is thought for DLVerification in case any of the area parameters (lat, lon, radius) are incorrect.

@jlurien
Copy link
Collaborator

jlurien commented Jan 22, 2025

In the current proposal, it is:

LOCATION_VERIFICATION.AREA_NOT_COVERED: The location is not supported -> This means for example that input area is out of the operator coverage (e.g. another country, ocean, etc)

LOCATION_VERIFICATION.INVALID_AREA: The area is too small, the polygon is too complex, etc -> This would be the error in case of the input area is too small to be accepted (for legal or implementation reasons)

@jlurien
Copy link
Collaborator

jlurien commented Jan 22, 2025

@maxl2287 To clarify the usage of these errors we may update in location verification, the following text in line #9:

API consumers are able to verify whether the location of certain user device is within the area specified. Currently the only area supported as input is a circle determined by a set of coordinates (latitude and longitude) and some expected accuracy (radius).

  • If the provided area is out of the operator's coverage or it is not supported for any reason, an error 422 LOCATION_VERIFICATION.AREA_NOT_COVERED will be returned.
  • Legal restrictions regarding privacy, or other regulatory or implementation issues, may force the operator to set restrictions to the provided area, such as setting a minimum value to the accepted radius. In these cases, an error 422 LOCATION_VERIFICATION.INVALID_AREA will be returned and the error message will refer to the reason of the limitation.

fyi @james-emerson @javier-carrocalabor. Please suggest any enhancements.

…mum-of-area-circle

# Conflicts:
#	code/API_definitions/location-verification.yaml
@maxl2287 maxl2287 dismissed stale reviews from jlurien and bigludo7 via 5cdd248 January 22, 2025 12:44
@maxl2287 maxl2287 requested review from jlurien and bigludo7 January 22, 2025 12:47
bigludo7
bigludo7 previously approved these changes Jan 22, 2025
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@james-emerson
Copy link

@javier-carrocalabor Thanks for your feedback to address my 1. Developer Guidance on Input Radius There is enough information for us to move forward with producing developer guidance for this release and we may revisit in future.

@maxl2287 Regarding my 2. Interpreting Error Messages I only have one concern with your proposal that is where it says "or implementation issues". This feels like a catch-all that does not relate to privacy or regulatory issues. Can this be more specific or put this under another error message?

@jlurien
Copy link
Collaborator

jlurien commented Jan 22, 2025

@maxl2287 Regarding my 2. Interpreting Error Messages I only have one concern with your proposal that is where it says "or implementation issues". This feels like a catch-all that does not relate to privacy or regulatory issues. Can this be more specific or put this under another error message?

Hi @james-emerson, I proposed this text. Adding "or implementation issues" tries to cover any constraint on the operator side (not necessarily regulatory pr privacy-related), that causes the implementation to reject the input area as invalid for the operator. The message can always give more detailed information. We may use another wording, or remove that part, but I wouldn't split codes for INVALID_AREA. wdyt @javier-carrocalabor ?

@maxl2287
Copy link
Contributor Author

Final changes were made. Please review @bigludo7 & @jlurien

Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maxl2287 maxl2287 merged commit 7c80a31 into camaraproject:main Jan 23, 2025
2 checks passed
@maxl2287 maxl2287 deleted the feature/decrease-minimum-of-area-circle branch January 23, 2025 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Spring25
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Verification/Geofencing]: Decrease the minimum of a Circle radius
5 participants