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

[ENH/FIX] Search clean-up #717

Merged
merged 3 commits into from
Nov 12, 2024
Merged

[ENH/FIX] Search clean-up #717

merged 3 commits into from
Nov 12, 2024

Conversation

rksh
Copy link
Contributor

@rksh rksh commented Nov 9, 2024

Here's a draft of that I've been working on. Notes in-line.

@@ -163,7 +163,7 @@ private void setupMap(final LatLng center, final Bundle savedInstanceState, fina
private void setupQuery( final QueryArgs queryArgs ) {

final LatLngBounds bounds = queryArgs.getLocationBounds();
String sql = "SELECT bssid,lastlat,lastlon FROM " + DatabaseHelper.NETWORK_TABLE + " WHERE 1=1 ";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe this whole method should move to DatabaseHelper, not just the query string?

this.bssid = ( bssid == null ) ? "" : bssid.toLowerCase(Locale.US);
this.ssid = ( ssid == null ) ? "" : ssid;
this.frequency = frequency;
this.capabilities = ( capabilities == null ) ? "" : capabilities;
this.level = level;
this.type = type;
if (null != lastTime && lastTime > 0L) {
this.lastTime = lastTime;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

HACK: Since the scheme is not-null, empty is resulting in zeros - I'd rather leave last null.

private static final SimpleDateFormat timeFormatter = new SimpleDateFormat(timePattern, l);
private static final SimpleDateFormat dateTimeFormatter = new SimpleDateFormat(dateTimePattern, l);
private static final SimpleDateFormat timeFormatter24 = new SimpleDateFormat(timePattern24, l);
private static final SimpleDateFormat dateTimeFormatter24 = new SimpleDateFormat(dateTimePattern24, l);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we were building these dynamically each time before, which seems silly. OTOH, now we're stuck building two instances we'll never use (over the entire app) because static initialization doesn't have access to Context we need to discriminate between 24 and 12 hour time preference.

public static String getTime(@NonNull final Network network, @NonNull final Context context) {
final Long last = network.getLastTime();
if (null == last) {
if (DateFormat.is24HourFormat(context)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

they made a utility method for us

} else {
return timeFormatter.format(new Date(network.getConstructionTime()));
}
// SOMEDAY (SDK26+: return Instant.ofEpochSecond(network.getConstructionTime()).atZone(ZoneId.systemDefault()).format(DateTimeFormatter.ofPattern(timePattern));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this would be great, but it's Android SDK 26+ so we'll use it when we get forced another two versions forward.

format = new SimpleDateFormat("H:mm:ss", Locale.getDefault());
public static String getTime(@NonNull final Network network, @NonNull final Context context) {
final Long last = network.getLastTime();
if (null == last) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if lasttime is null then use construction time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has a practical problem - if lasttime is null (genuinely surprised at how common this is proving), then you get construction time.

Copy link
Member

@bobzilladev bobzilladev left a comment

Choose a reason for hiding this comment

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

all seems reasonable to this caveman programmer

@@ -144,6 +144,8 @@ public final class DatabaseHelper extends Thread {
private static final String LOCATED_NETS_QUERY_STEM = " FROM " + DatabaseHelper.NETWORK_TABLE
+ " WHERE bestlat != 0.0 AND bestlon != 0.0 AND instr(bssid, '_') <= 0";

public static final String SEARCH_NETWORKS = "SELECT bssid,lastlat,lastlon FROM " + NETWORK_TABLE + " WHERE 1=1 ";
//TODO: should search use best[lat|lon] instead of last?
Copy link
Member

Choose a reason for hiding this comment

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

Maybe any time we only show a single location it should be the 'best' one. It's newer, back in the day we didn't have best.

@rksh rksh removed the do not merge label Nov 12, 2024
@rksh rksh marked this pull request as ready for review November 12, 2024 20:35
@rksh rksh merged commit eede48c into wiglenet:main Nov 12, 2024
1 check passed
@rksh rksh deleted the search_fixes branch November 14, 2024 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants