Skip to content

[feature] Add API endpoint for indoor map coordinates #828#976

Merged
nemesifier merged 20 commits intoopenwisp:gsoc25-mapfrom
dee077:feature/828-api-indoor-map-coordinates
Jul 29, 2025
Merged

[feature] Add API endpoint for indoor map coordinates #828#976
nemesifier merged 20 commits intoopenwisp:gsoc25-mapfrom
dee077:feature/828-api-indoor-map-coordinates

Conversation

@dee077
Copy link
Member

@dee077 dee077 commented Feb 2, 2025

Implemented API to return device coordinates for a given location ID.

Fixes #828

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue

Closes #828

Description of Changes

  • Added endpoint floorplan_coordinates_list to list coordinates
  • Created a new serializer to add this below fields
fields = [
    "name", 
    "mac_address",
     "is_deactivated",
     "model",
     "os",
     "floor_name",
     "floor",
     "image",
     "location",
]
  • Added filter by floor
  • Added pagination class
  • Added test

@dee077 dee077 force-pushed the feature/828-api-indoor-map-coordinates branch 3 times, most recently from 103a39c to 611f852 Compare February 2, 2025 17:22
@coveralls
Copy link

coveralls commented Feb 2, 2025

Coverage Status

coverage: 98.888% (-0.01%) from 98.899%
when pulling 1754156 on dee077:feature/828-api-indoor-map-coordinates
into 129a42f on openwisp:master.

@dee077 dee077 closed this Feb 2, 2025
@dee077 dee077 reopened this Feb 2, 2025
@dee077 dee077 changed the title [feature] Add API endpoint for indoor map coordinates #828 [feature] Add API endpoint for indoor map coordinates Feb 2, 2025
@dee077 dee077 changed the title [feature] Add API endpoint for indoor map coordinates [feature] Add API endpoint for indoor map coordinates #828 Feb 2, 2025
@pandafy pandafy self-requested a review February 3, 2025 08:59
@nemesifier
Copy link
Member

@dee077 Isn't this part of the GSoC project idea for the indoor map?

@dee077
Copy link
Member Author

dee077 commented Feb 7, 2025

Yes, this aligns with the GSoC project idea for the indoor map. However, at the time, it didn’t have the gsoc-idea label, so I wasn’t aware that it would be proposed as a GSoC idea for 2025.

@dee077 dee077 changed the base branch from master to gsoc25-map May 23, 2025 14:53
@nemesifier nemesifier added the gsoc Part of a Google Summer of Code project label May 23, 2025
@dee077 dee077 force-pushed the feature/828-api-indoor-map-coordinates branch 4 times, most recently from d3ca54d to e1e9295 Compare June 11, 2025 13:40
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

The output looks very similar to GeoJSON, but it's not.
Good progress @dee077 👏

Look at where we use GeoJSON (rest_framework_gis): https://github.com/search?q=repo%3Aopenwisp%2Fopenwisp-controller%20rest_framework_gis&type=code.

Let's paginate the results and ensure the frontend can load paginated results gradually without crashing if there's many floorplan points scattered on many floors.

@github-project-automation github-project-automation bot moved this from To do (general) to In progress in OpenWISP Contributor's Board Jun 14, 2025
@dee077 dee077 force-pushed the feature/828-api-indoor-map-coordinates branch from c9ab3be to f6bf373 Compare June 16, 2025 18:15
@dee077 dee077 marked this pull request as ready for review June 17, 2025 17:14
@dee077 dee077 force-pushed the feature/828-api-indoor-map-coordinates branch from bafe7c1 to 0b15b71 Compare June 17, 2025 17:26
@dee077 dee077 moved this from In progress to In review in [GSoC25] General Map: Indoor, Mobile, Linkable URLs Jun 18, 2025
@dee077 dee077 force-pushed the feature/828-api-indoor-map-coordinates branch 2 times, most recently from d77d518 to 69976d0 Compare June 26, 2025 22:24
@dee077
Copy link
Member Author

dee077 commented Jul 16, 2025

Updates

  • Updated filter class as suggested.
  • Updated docs
  • Updated tests to use more descriptive variable names

@dee077 dee077 force-pushed the feature/828-api-indoor-map-coordinates branch from 319676a to eec111f Compare July 16, 2025 23:12
Copy link
Member

@pandafy pandafy left a comment

Choose a reason for hiding this comment

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

@dee077 I found some minor issues on this PR which I missed in my previous review.

Since, all of these changes are minor, I will take care of them now so you can focus working on other tasks.

However, I ask you to review all the comments so we can avoid them in the future.


.. note::

this endpoint returns device coordinates from the first floor above
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this endpoint returns device coordinates from the first floor above
This endpoint returns device coordinates from the first floor above

Comment on lines +379 to +382
def get_floor_name(self, obj):
loc_name = obj.floorplan.location.name
floor_no = obj.floorplan.floor
return f"{loc_name} {ordinal(floor_no)} Floor"
Copy link
Member

Choose a reason for hiding this comment

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

I thought, I already respond to this question, my bad.

The django-loci model has the correct string representation for the object. Can you we that?

https://github.com/openwisp/django-loci/blob/4e45c8d986697cf851bc2b04bbb01b989c63720f/django_loci/base/models.py#L101-L106

Comment on lines +246 to +247
def get_parent_queryset(self):
return DeviceLocation.objects.filter(location__pk=self.kwargs.get("pk"))
Copy link
Member

Choose a reason for hiding this comment

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

The get_parent_queryset() method is used to get the list of parent elements which has the organization field.

https://github.com/openwisp/openwisp-users/blob/17aeffb5be731c5c969475bad5690bb94711c0fd/openwisp_users/api/mixins.py#L108-L115

I believe returning the queryset of the Location object will be more appropriate here.

Right now, both parent and child models are same.

filter_backends = [filters.DjangoFilterBackend]
filterset_class = IndoorCoordinatesFilter
pagination_class = IndoorCoodinatesViewPagination
organization_field = "content_object__organization"
Copy link
Member

Choose a reason for hiding this comment

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

While this works without any issues, for logical reasons I think `floorplan__organization" will be more appropriate here.

self.assertEqual(r.status_code, 401)

@capture_any_output()
def test_indoor_coodinate_list(self):
Copy link
Member

Choose a reason for hiding this comment

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

There's a lack of check which verifies the complete response.

Ideally, we want the tests to fail when we make changes to the code. Right now, the tests are not exhaustive. E.g., if I remove the admin url from the response, then none of the tests would fail.

r = self.client.get(reverse(url))
self.assertEqual(r.status_code, 401)

@capture_any_output()
Copy link
Member

Choose a reason for hiding this comment

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

@dee077 why did you use capture_any_output for this test?

We generally use this to suppress noisy logging while running the test suite. In my testing this test did not generate any noise.


class IndoorCoordinatesSerializer(serializers.ModelSerializer):
admin_edit_url = SerializerMethodField("get_admin_edit_url")
content_object_id = serializers.UUIDField(
Copy link
Member

Choose a reason for hiding this comment

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

I think, device_id would be more descriptive here instead of content_object_id.

This endpoint returns device coordinates from the first floor above
ground (lowest non-negative floors) by default. If a location only has
negative floors (e.g. underground parking lot), then it will return
the closest floor to the ground (maximum negative floor).
Copy link
Member

@nemesifier nemesifier Jul 29, 2025

Choose a reason for hiding this comment

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

Suggested change
the closest floor to the ground (maximum negative floor).
the closest floor to the ground (greatest negative floor).

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated!

name="detail_location",
),
path(
"api/v1/controller/location/<str:pk>/indoor-coordinates/",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"api/v1/controller/location/<str:pk>/indoor-coordinates/",
"api/v1/controller/location/<uuid:pk>/indoor-coordinates/",

Please verify that trying to open an URL with an invalid UUID returns 404 (should be automatically handled by Django), if that's not the case please write a test for this and fix the bug (hopefully not necessary).

Copy link
Member

Choose a reason for hiding this comment

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

PS: you have copied the URL definition structure from the URLs above, which also suffer from the same issue, I am fixing that in #1101.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to uuid and it is showing 404 not found on invalid id

image

@dee077 dee077 force-pushed the feature/828-api-indoor-map-coordinates branch from d39ba0e to 8e81084 Compare July 29, 2025 22:35
@nemesifier nemesifier merged commit daa9ed3 into openwisp:gsoc25-map Jul 29, 2025
12 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in OpenWISP Contributor's Board Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gsoc Part of a Google Summer of Code project

Development

Successfully merging this pull request may close these issues.

[feature] Add API endpoint that returns co-ordinates on indoor map (floorplan)

4 participants