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

Fix #162 - Add showNegativeScheduledArrivals option to data-sources.xml #160

Closed

Conversation

cagryInside
Copy link
Contributor

I have implemented showNegativeScheduledArrivals option in data-soruces.xml. Now we can set the property:

  <property name="showNegativeScheduledArrivals" value="false" />

It set default true if a user never sets showNegativeScheduledArrivals property.

cc @barbeau

@onebusawaybot
Copy link

Can one of the admins verify this patch?

@barbeau
Copy link
Member

barbeau commented Jan 1, 2016

@cagryInside Does this hide all negative arrivals (schedule and real-time times), or just negative arrivals for scheduled times when real-time isn't available? For the PSTA situation (which I still need to open an issue with an explanation for), we want to show negative arrivals for real-time data, but not for schedule data. So, we should only hide negative arrivals when no real-time info is available.

@barbeau
Copy link
Member

barbeau commented Jan 1, 2016

On a related note, we should also probably rename the property to showNegativeScheduledArrivals.

@cagryInside
Copy link
Contributor Author

@barbeau Yes this improvement hides all negative arrivals.
I check the negative arrivals by looking at the numberOfStopsAway property. Now, I should also check predictedArrivalTime. For example:

      if(!isNegativeArrivalsEnabled(instance.getBlockTrip().getTrip().getId().getAgencyId())
          && bean.getNumberOfStopsAway() < 0 && bean.getPredictedArrivalTime() <= 0)

But I am not sure this is the best approach the check predictedArrivalTime. Can we discuss this sometime this week?

@barbeau
Copy link
Member

barbeau commented Jan 4, 2016

Yes, let's discuss this week.

@cagryInside cagryInside force-pushed the develop_negative_arrivals branch 2 times, most recently from e1c810a to 7fa55cc Compare January 7, 2016 18:13
@barbeau barbeau changed the title WIP - Add showNegativeArrivals option to data-sources.xml WIP - Add showNegativeScheduledArrivals option to data-sources.xml Jan 7, 2016
@barbeau
Copy link
Member

barbeau commented Jan 7, 2016

Per our discussion, bean.getPredictedArrivalTime() <= 0 is a good indicator of whether or not real-time info exists, so that portion looks fine.

@barbeau
Copy link
Member

barbeau commented Jan 7, 2016

We're doing some final tests to make sure that bean.getNumberOfStopsAway() < 0 is a good indicator of whether or not the arrival is scheduled to have occurred in the past. If so, then everything should be working correctly. If not, we'll need to come up with another implementation to check if the scheduled time (arrival or departure, depending on the stop_sequence) is in the past.

@barbeau
Copy link
Member

barbeau commented Jan 7, 2016

The issue describing the problem this PR addresses is #162.

@barbeau barbeau changed the title WIP - Add showNegativeScheduledArrivals option to data-sources.xml WIP for #162 - Add showNegativeScheduledArrivals option to data-sources.xml Jan 7, 2016
@barbeau
Copy link
Member

barbeau commented Jan 20, 2016

This patch is currently deployed to http://app.staging.obahart.org/ for the agency PSTA:
http://app.staging.obahart.org/where/standard/stop.action?id=PSTA_4025

@barbeau barbeau changed the title WIP for #162 - Add showNegativeScheduledArrivals option to data-sources.xml Fix #162 - Add showNegativeScheduledArrivals option to data-sources.xml Jan 20, 2016
@barbeau
Copy link
Member

barbeau commented Jan 20, 2016

@sheldonabrown I believe this should be ready for review. We've been testing it with PSTA and it seems to be working as intended.

@sheldonabrown
Copy link
Member

Merged this into camsys/unified branch, with intent to later merge camsys/unified to onebusaway/master

@aaronbrethorst
Copy link
Member

no action in 8 years. closing.

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.

None yet

5 participants