Skip to content

Fix #1537: Starred stops follow-up : layout dedup, error state, parallel requests #1552

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.

Sign up for GitHub

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

Open
diveshpatil9104 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix #1537: Starred stops follow-up : layout dedup, error state, parallel requests #1552

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 24 additions & 2 deletions onebusaway-android/src/main/java/org/onebusaway/android/ui/StarredStopsAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import android.database.Cursor;
import android.graphics.drawable.GradientDrawable;
import android.view.View;
import android.widget.ImageView;
import android.widget.HorizontalScrollView;
import android.widget.ImageView;
import android.widget.LinearLayout;
import android.widget.ProgressBar;
import android.widget.TextView;
Expand All @@ -37,6 +37,8 @@

/**
* Adapter for starred stops list that displays arrival badges inline.
* A null value in the arrivals map indicates a fetch error for that stop,
* while an empty list means no upcoming arrivals.
*/
class StarredStopsAdapter extends SimpleCursorAdapter {

Expand Down Expand Up @@ -104,22 +106,42 @@ public void bindView(View view, Context context, Cursor cursor) {
HorizontalScrollView arrivalsScroll = view.findViewById(R.id.arrivals_scroll);
LinearLayout arrivalsContainer = view.findViewById(R.id.arrivals_container);
ProgressBar arrivalsLoading = view.findViewById(R.id.arrivals_loading);
TextView arrivalsError = view.findViewById(R.id.arrivals_error);

if (mArrivalsData == null) {
// Still loading
arrivalsScroll.setVisibility(View.GONE);
arrivalsError.setVisibility(View.GONE);
arrivalsLoading.setVisibility(View.VISIBLE);
return;
}

arrivalsLoading.setVisibility(View.GONE);

if (!mArrivalsData.containsKey(stopId)) {
arrivalsScroll.setVisibility(View.GONE);
arrivalsError.setVisibility(View.GONE);
return;
}

ArrayList<ArrivalInfo> arrivals = mArrivalsData.get(stopId);

if (arrivals == null || arrivals.isEmpty()) {
// Null value means fetch error
if (arrivals == null) {
arrivalsScroll.setVisibility(View.GONE);
arrivalsError.setVisibility(View.VISIBLE);
arrivalsError.setText(R.string.starred_stop_arrivals_error);
return;
}

// Empty list means no upcoming arrivals
if (arrivals.isEmpty()) {
arrivalsScroll.setVisibility(View.GONE);
arrivalsError.setVisibility(View.GONE);
return;
}

arrivalsError.setVisibility(View.GONE);
arrivalsScroll.setVisibility(View.VISIBLE);
arrivalsContainer.removeAllViews();

Expand Down
108 changes: 86 additions & 22 deletions onebusaway-android/src/main/java/org/onebusaway/android/ui/StarredStopsArrivalsLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,18 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;

import androidx.loader.content.AsyncTaskLoader;

/**
* Loader that fetches arrival info for multiple starred stops in the background.
* Loader that fetches arrival info for multiple starred stops in parallel.
* Uses a null value in the result map to indicate a fetch error for a stop,
* distinguishing it from an empty list (no upcoming arrivals).
*/
public class StarredStopsArrivalsLoader
extends AsyncTaskLoader<HashMap<String, ArrayList<ArrivalInfo>>> {
Expand All @@ -41,6 +48,8 @@ public class StarredStopsArrivalsLoader

private static final int MAX_ARRIVALS_PER_STOP = 3;

private static final int THREAD_POOL_SIZE = 3;

private final String[] mStopIds;

private HashMap<String, ArrayList<ArrivalInfo>> mResult;
Expand All @@ -55,32 +64,29 @@ public HashMap<String, ArrayList<ArrivalInfo>> loadInBackground() {
HashMap<String, ArrayList<ArrivalInfo>> result = new HashMap<>();
long now = System.currentTimeMillis();

if (mStopIds.length == 0) {
return result;
}

ExecutorService executor = Executors.newFixedThreadPool(
Math.min(THREAD_POOL_SIZE, mStopIds.length));
List<Future<StopArrivalsResult>> futures = new ArrayList<>();

for (String stopId : mStopIds) {
ArrayList<ArrivalInfo> arrivals = new ArrayList<>();
futures.add(executor.submit(new FetchArrivalsTask(getContext(), stopId, now)));
}

for (int i = 0; i < mStopIds.length; i++) {
try {
ObaArrivalInfoResponse response =
ObaArrivalInfoRequest.newRequest(getContext(), stopId, MINUTES_AFTER)
.call();
if (response != null && response.getCode() == ObaApi.OBA_OK
&& response.getArrivalInfo() != null) {
arrivals = ArrivalInfoUtils.convertObaArrivalInfo(
getContext(), response.getArrivalInfo(), null, now, false);
Collections.sort(arrivals, new ArrivalInfoUtils.InfoComparator());
if (arrivals.size() > MAX_ARRIVALS_PER_STOP) {
arrivals = new ArrayList<>(
arrivals.subList(0, MAX_ARRIVALS_PER_STOP));
}
} else if (response != null) {
Log.w(TAG, "API error for stop " + stopId
+ ": code=" + response.getCode());
} else {
Log.w(TAG, "Null response for stop " + stopId);
}
StopArrivalsResult stopResult = futures.get(i).get();
result.put(stopResult.stopId, stopResult.arrivals);
} catch (Exception e) {
Log.w(TAG, "Failed to fetch arrivals for stop " + stopId, e);
Log.w(TAG, "Failed to get result for stop " + mStopIds[i], e);
result.put(mStopIds[i], null);
}
result.put(stopId, arrivals);
}

executor.shutdown();
return result;
}

Expand Down Expand Up @@ -113,4 +119,62 @@ protected void onReset() {
onStopLoading();
mResult = null;
}

private static class StopArrivalsResult {

final String stopId;

final ArrayList<ArrivalInfo> arrivals;

StopArrivalsResult(String stopId, ArrayList<ArrivalInfo> arrivals) {
this.stopId = stopId;
this.arrivals = arrivals;
}
}

private static class FetchArrivalsTask implements Callable<StopArrivalsResult> {

private final Context mContext;

private final String mStopId;

private final long mNow;

FetchArrivalsTask(Context context, String stopId, long now) {
mContext = context;
mStopId = stopId;
mNow = now;
}

@Override
public StopArrivalsResult call() {
ArrayList<ArrivalInfo> arrivals = new ArrayList<>();
try {
ObaArrivalInfoResponse response =
ObaArrivalInfoRequest.newRequest(mContext, mStopId, MINUTES_AFTER)
.call();
if (response != null && response.getCode() == ObaApi.OBA_OK
&& response.getArrivalInfo() != null) {
arrivals = ArrivalInfoUtils.convertObaArrivalInfo(
mContext, response.getArrivalInfo(), null, mNow, false);
Collections.sort(arrivals, new ArrivalInfoUtils.InfoComparator());
if (arrivals.size() > MAX_ARRIVALS_PER_STOP) {
arrivals = new ArrayList<>(
arrivals.subList(0, MAX_ARRIVALS_PER_STOP));
}
} else if (response != null) {
Log.w(TAG, "API error for stop " + mStopId
+ ": code=" + response.getCode());
return new StopArrivalsResult(mStopId, null);
} else {
Log.w(TAG, "Null response for stop " + mStopId);
return new StopArrivalsResult(mStopId, null);
}
} catch (Exception e) {
Log.w(TAG, "Failed to fetch arrivals for stop " + mStopId, e);
return new StopArrivalsResult(mStopId, null);
}
return new StopArrivalsResult(mStopId, arrivals);
}
}
}
96 changes: 35 additions & 61 deletions onebusaway-android/src/main/res/layout/starred_stop_list_item.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,70 +15,44 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
style="@style/ListItem"
android:id="@+id/stop_list_container"
android:layout_height="wrap_content"
android:layout_width="fill_parent"
android:minHeight="77dp"
android:orientation="horizontal">
<ImageView
android:id="@+id/stop_favorite"
android:src="@drawable/ic_toggle_star"
android:contentDescription="@string/stop_info_favorite"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:scaleType="fitCenter"
android:adjustViewBounds="true"
android:maxHeight="25dp"
android:maxWidth="25dp"
android:layout_marginLeft="5dp"
android:layout_centerVertical="true"
android:layout_alignParentLeft="true"
android:visibility="gone"/>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="fill_parent"
android:layout_height="wrap_content"
android:orientation="vertical">

<LinearLayout
style="@style/ListItem"
android:layout_marginLeft="@dimen/keyline_2"
android:layout_alignParentLeft="true"
android:layout_centerVertical="true"
android:layout_width="fill_parent"
<include layout="@layout/stop_list_item"/>

<HorizontalScrollView
android:id="@+id/arrivals_scroll"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="vertical">
<TextView
android:id="@+id/stop_name"
style="@style/Line1Text"
android:layout_marginLeft="@dimen/keyline_2"
android:layout_marginBottom="8dp"
android:scrollbars="none"
android:visibility="gone">
<LinearLayout
android:id="@+id/arrivals_container"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_vertical"
android:ellipsize="end"
android:maxLines="1"/>
<TextView
android:id="@+id/direction"
style="@style/Line2Text"
android:layout_width="wrap_content"
android:layout_height="wrap_content"/>
android:orientation="horizontal"/>
</HorizontalScrollView>

<HorizontalScrollView
android:id="@+id/arrivals_scroll"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="4dp"
android:scrollbars="none"
android:visibility="gone">
<LinearLayout
android:id="@+id/arrivals_container"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:orientation="horizontal"/>
</HorizontalScrollView>
<ProgressBar
android:id="@+id/arrivals_loading"
style="?android:attr/progressBarStyleSmall"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginLeft="@dimen/keyline_2"
android:layout_marginBottom="8dp"
android:visibility="gone"/>

<ProgressBar
android:id="@+id/arrivals_loading"
style="?android:attr/progressBarStyleSmall"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="4dp"
android:visibility="gone"/>
</LinearLayout>
</RelativeLayout>
<TextView
android:id="@+id/arrivals_error"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginLeft="@dimen/keyline_2"
android:layout_marginBottom="8dp"
android:textSize="11sp"
android:textColor="?android:attr/textColorSecondary"
android:visibility="gone"/>
</LinearLayout>
1 change: 1 addition & 0 deletions onebusaway-android/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@
<string name="starred_stop_arrival_now">NOW</string>
<string name="starred_stop_arrival_min">%1$d min</string>
<string name="starred_stop_arrival_badge">%1$s - %2$s</string>
<string name="starred_stop_arrivals_error">Could not load arrivals</string>
<string name="my_no_starred_routes">You have no starred routes.\n\nYou can add a star to a route
on the arrivals screen by tapping on the star next to the route number.
</string>
Expand Down
Loading