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

📍 Use the server-side geocoded addresses for place names #1166

Merged
merged 1 commit into from
Aug 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions www/js/diary/addressNamesHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,23 @@ import Bottleneck from 'bottleneck';
import { displayError, logDebug } from '../plugin/logger';
import { CompositeTrip } from '../types/diaryTypes';

export type NominatimResponse = {
address: {
road?: string;
pedestrian?: string;
suburb?: string;
neighbourhood?: string;
hamlet?: string;
city?: string;
town?: string;
county?: string;
state?: string;
postcode?: string;
country?: string;
country_code?: string;
};
};

let nominatimLimiter = new Bottleneck({ maxConcurrent: 2, minTime: 500 });
export const resetNominatimLimiter = () => {
const newLimiter = new Bottleneck({ maxConcurrent: 2, minTime: 500 });
Expand Down Expand Up @@ -128,7 +145,7 @@ async function fetchNominatimLocName(loc_geojson) {
status = ${response.status};
data = ${JSON.stringify(data)}`);
localStorage.setItem(coordsStr, JSON.stringify(data));
publish(coordsStr, data);
publish(coordsStr, JSON.stringify(data));
} catch (error) {
if (!nominatimError) {
nominatimError = error;
Expand All @@ -139,8 +156,16 @@ async function fetchNominatimLocName(loc_geojson) {

// Schedules nominatim fetches for the start and end locations of a trip
export function fillLocationNamesOfTrip(trip: CompositeTrip) {
nominatimLimiter.schedule(() => fetchNominatimLocName(trip.end_loc));
nominatimLimiter.schedule(() => fetchNominatimLocName(trip.start_loc));
[trip.start_confirmed_place, trip.end_confirmed_place].forEach((place) => {
if (place?.geocoded_address || place?.reverse_geocode) {
const coordsStr = place.location.coordinates.toString();
const data = place.reverse_geocode || { address: place.geocoded_address };
localStorage.setItem(coordsStr, JSON.stringify(data));
publish(coordsStr, JSON.stringify(data));
Comment on lines +162 to +164
Copy link
Contributor

Choose a reason for hiding this comment

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

nice! great idea to reuse the data from prior mappings even for trips that don't have the address instead of looking it up.

} else {
nominatimLimiter.schedule(() => fetchNominatimLocName(place.location));
}
});
}

// a React hook that takes a trip or place and returns an array of its address names
Expand Down
12 changes: 9 additions & 3 deletions www/js/types/diaryTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
and user input objects.
As much as possible, these types parallel the types used in the server code. */

import { NominatimResponse } from '../diary/addressNamesHelper';
import { BaseModeKey, MotionTypeKey } from '../diary/diaryHelper';
import useDerivedProperties from '../diary/useDerivedProperties';
import { VehicleIdentity } from './appConfigTypes';
Expand Down Expand Up @@ -32,8 +33,13 @@ export type ConfirmedPlace = {
exit_fmt_time: string; // ISO string e.g. 2023-10-31T12:00:00.000-04:00
exit_local_dt: LocalDt;
exit_ts: number; // Unix timestamp

// one of these depending on what we decide to keep on the server
geocoded_address?: NominatimResponse['address'];
reverse_geocode?: NominatimResponse;

key: string;
location: Geometry;
location: Point;
origin_key: string;
raw_places: ObjectId[];
source: string;
Expand Down Expand Up @@ -96,7 +102,7 @@ export type CompositeTrip = {
confirmed_trip: ObjectId;
distance: number;
duration: number;
end_confirmed_place: BEMData<ConfirmedPlace>;
end_confirmed_place: ConfirmedPlace;
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious about what this change does. Why did we have BEMData before and why don't we need it now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was just incorrect. I noticed because Typescript complained when I accessed properties on the place objects while making the changes

Updated types:

-Since the CompositeTrip type definition is based on the return type of Timeline.readAllCompositeTrips, which is after "unpacking", start_confirmed_place and end_confirmed_place are simply ConfirmedPlace, not BEMData
-ConfirmedPlace.location is a Point, (having coordinates) not just a generic Geometry
-added geocoded_address and reverse_geocode, both as optional since it depends on e-mission/e-mission-server#973

end_fmt_time: string;
end_loc: Point;
end_local_dt: LocalDt;
Expand All @@ -113,7 +119,7 @@ export type CompositeTrip = {
raw_trip: ObjectId;
sections: SectionData[];
source: string;
start_confirmed_place: BEMData<ConfirmedPlace>;
start_confirmed_place: ConfirmedPlace;
start_fmt_time: string;
start_loc: Point;
start_local_dt: LocalDt;
Expand Down
Loading