Skip to content

Commit

Permalink
Deprecate methods in the Router interface
Browse files Browse the repository at this point in the history
The methods selectRoute(), getSelectedRoute() and getTargetedPoints()
in the Router interface are now deprecated because they have not been
used or do not belong to the routers designated tasks and therefore were
outsourced to the classes where the logic belongs.

Merged-by: Martin Grzenia <[email protected]>
  • Loading branch information
Finja Averhaus authored and martingr committed Nov 22, 2024
1 parent f46379d commit 5743901
Show file tree
Hide file tree
Showing 16 changed files with 237 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,11 @@ long getCosts(
* @param driveOrders The drive orders encapsulating the route being selected,
* or <code>null</code>, if no route is being selected for the vehicle (i.e.
* an existing entry for the given vehicle would be removed).
* @deprecated Will be removed without replacement. A vehicle's selected route (i.e. the list of
* drive orders) is already contained in the transport order assigned to it.
*/
@Deprecated
@ScheduledApiChange(when = "7.0", details = "Will be removed.")
void selectRoute(
@Nonnull
Vehicle vehicle,
Expand All @@ -216,7 +220,11 @@ void selectRoute(
* </p>
*
* @return An unmodifiable view on the selected routes the router knows about.
* @deprecated Will be removed without replacement. A vehicle's selected route (i.e. the list of
* drive orders) is already contained in the transport order assigned to it.
*/
@Deprecated
@ScheduledApiChange(when = "7.0", details = "Will be removed.")
@Nonnull
Map<Vehicle, List<DriveOrder>> getSelectedRoutes();

Expand All @@ -227,7 +235,11 @@ void selectRoute(
* </p>
*
* @return A set of all points currently targeted by any vehicle.
* @deprecated Will be removed without replacement. The points targeted by vehicles can be
* retrieved via the transport orders assigned to them.
*/
@Deprecated
@ScheduledApiChange(when = "7.0", details = "Will be removed.")
@Nonnull
Set<Point> getTargetedPoints();
}
2 changes: 2 additions & 0 deletions opentcs-documentation/src/docs/release-notes/changelog.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ This change log lists the most relevant changes for past releases in reverse chr
** Avoid a `NullPointerException` when trying to park a vehicle whose current position is not known.
** Ensure vehicles can process newly assigned transport orders after a peripheral job (created in the context of a previous transport order) has failed.
Previously, failed peripheral jobs with the `AFTER_ALLOCATION` execution trigger could prevent vehicles from properly processing transport orders in some situations.
* Changes affecting developers:
** Deprecate methods in the `Router` interface that are technically outside its scope.

== Version 6.2 (2024-11-06)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,6 @@ private void updateTransportOrder(
transportOrderService.fetchObject(TransportOrder.class, originalOrder.getReference())
);
}

// Let the router know the vehicle selected another route
router.selectRoute(vehicle, newOrders);
}

private List<DriveOrder> updatePathLocks(List<DriveOrder> orders) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import jakarta.annotation.Nonnull;
import jakarta.inject.Inject;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import org.opentcs.components.Lifecycle;
Expand Down Expand Up @@ -213,8 +212,6 @@ public void assignTransportOrder(
.updateOrderSequenceProcessingVehicle(transportOrder.getWrappingSequence(), vehicleRef);
}
transportOrderService.updateTransportOrderProcessingVehicle(orderRef, vehicleRef, driveOrders);
// Let the router know about the route chosen.
router.selectRoute(vehicle, Collections.unmodifiableList(driveOrders));
// Update the transport order's copy.
TransportOrder updatedOrder = transportOrderService.fetchObject(TransportOrder.class, orderRef);
// If the drive order must be assigned, do so.
Expand Down Expand Up @@ -324,9 +321,6 @@ private void finishAbortion(TCSObjectReference<TransportOrder> orderRef, Vehicle

vehicleService.updateVehicleProcState(vehicle.getReference(), Vehicle.ProcState.IDLE);
vehicleService.updateVehicleTransportOrder(vehicle.getReference(), null);

// Let the router know that the vehicle doesn't have a route any more.
router.selectRoute(vehicle, null);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// SPDX-FileCopyrightText: The openTCS Authors
// SPDX-License-Identifier: MIT
package org.opentcs.strategies.basic.dispatching.phase;

import static java.util.Objects.requireNonNull;

import jakarta.inject.Inject;
import java.util.Set;
import java.util.stream.Collectors;
import org.opentcs.components.kernel.services.TCSObjectService;
import org.opentcs.data.model.Point;
import org.opentcs.data.order.TransportOrder;

/**
* A point supplier that tries to find all points which are currently targeted by vehicles.
*/
public class TargetedPointsSupplier {

/**
* The object service.
*/
private final TCSObjectService objectService;

@Inject
public TargetedPointsSupplier(TCSObjectService objectService) {
this.objectService = requireNonNull(objectService, "objectService");
}

/**
* Returns all points which are currently targeted by vehicles.
*
* @return A set of all points currently targeted by vehicles.
*/
public Set<Point> getTargetedPoints() {
return objectService
.fetchObjects(
TransportOrder.class,
order -> order.hasState(TransportOrder.State.BEING_PROCESSED)
)
.stream()
.map(
transportOrder -> transportOrder.getAllDriveOrders().getLast().getRoute()
.getFinalDestinationPoint()
)
.collect(Collectors.toSet());

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,6 @@ private void checkForNextDriveOrder(Vehicle vehicle) {
// Update the vehicle's procState, implicitly dispatching it again.
vehicleService.updateVehicleProcState(vehicle.getReference(), Vehicle.ProcState.IDLE);
vehicleService.updateVehicleTransportOrder(vehicle.getReference(), null);
// Let the router know that the vehicle doesn't have a route any more.
router.selectRoute(vehicle, null);
// Update transport orders that are dispatchable now that this one has been finished.
transportOrderUtil.markNewDispatchableOrders();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.opentcs.components.kernel.services.InternalPlantModelService;
import org.opentcs.data.model.Point;
import org.opentcs.data.model.Vehicle;
import org.opentcs.strategies.basic.dispatching.phase.TargetedPointsSupplier;

/**
* An abstract base class for parking position suppliers.
Expand All @@ -29,6 +30,10 @@ public abstract class AbstractParkingPositionSupplier
* A router for computing distances to parking positions.
*/
private final Router router;
/**
* Finds all points which are currently targeted by vehicles.
*/
private final TargetedPointsSupplier targetedPointsSupplier;
/**
* Indicates whether this component is initialized.
*/
Expand All @@ -39,13 +44,16 @@ public abstract class AbstractParkingPositionSupplier
*
* @param plantModelService The plant model service.
* @param router A router for computing distances to parking positions.
* @param targetedPointsSupplier Finds all points which are currently targeted by vehicles.
*/
protected AbstractParkingPositionSupplier(
InternalPlantModelService plantModelService,
Router router
Router router,
TargetedPointsSupplier targetedPointsSupplier
) {
this.plantModelService = requireNonNull(plantModelService, "plantModelService");
this.router = requireNonNull(router, "router");
this.targetedPointsSupplier = requireNonNull(targetedPointsSupplier, "targetedPointsSupplier");
}

@Override
Expand Down Expand Up @@ -97,12 +105,12 @@ public Router getRouter() {
* @return The set of usable parking positions.
*/
protected Set<Point> findUsableParkingPositions(Vehicle vehicle) {
// Find out which points are destination points of the current routes of
// all vehicles, and keep them. (Multiple lookups ahead.)
Set<Point> targetedPoints = getRouter().getTargetedPoints();

return fetchAllParkingPositions().stream()
.filter(point -> isPointUnoccupiedFor(point, vehicle, targetedPoints))
.filter(
point -> isPointUnoccupiedFor(
point, vehicle, targetedPointsSupplier.getTargetedPoints()
)
)
.collect(Collectors.toSet());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.opentcs.components.kernel.services.InternalPlantModelService;
import org.opentcs.data.model.Point;
import org.opentcs.data.model.Vehicle;
import org.opentcs.strategies.basic.dispatching.phase.TargetedPointsSupplier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -36,13 +37,15 @@ public class DefaultParkingPositionSupplier
*
* @param plantModelService The plant model service.
* @param router A router for computing travel costs to parking positions.
* @param targetedPointsSupplier Finds all points which are currently targeted by vehicles.
*/
@Inject
public DefaultParkingPositionSupplier(
InternalPlantModelService plantModelService,
Router router
Router router,
TargetedPointsSupplier targetedPointsSupplier
) {
super(plantModelService, router);
super(plantModelService, router, targetedPointsSupplier);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.opentcs.components.kernel.services.InternalPlantModelService;
import org.opentcs.data.model.Point;
import org.opentcs.data.model.Vehicle;
import org.opentcs.strategies.basic.dispatching.phase.TargetedPointsSupplier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -44,14 +45,16 @@ public class PrioritizedParkingPositionSupplier
* @param plantModelService The plant model service.
* @param router A router for computing travel costs to parking positions.
* @param priorityFunction A function computing the priority of a parking position.
* @param targetedPointsSupplier Returns all points which are currently targeted by vehicles.
*/
@Inject
public PrioritizedParkingPositionSupplier(
InternalPlantModelService plantModelService,
Router router,
ParkingPositionToPriorityFunction priorityFunction
ParkingPositionToPriorityFunction priorityFunction,
TargetedPointsSupplier targetedPointsSupplier
) {
super(plantModelService, router);
super(plantModelService, router, targetedPointsSupplier);
this.priorityFunction = requireNonNull(priorityFunction, "priorityFunction");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.opentcs.data.model.Point;
import org.opentcs.data.model.Vehicle;
import org.opentcs.data.order.DriveOrder;
import org.opentcs.strategies.basic.dispatching.phase.TargetedPointsSupplier;

/**
* Finds assigned, preferred or (routing-wise) cheapest recharge locations for vehicles.
Expand All @@ -38,6 +39,10 @@ public class DefaultRechargePositionSupplier
* Our router.
*/
private final Router router;
/**
* Finds all points which are currently targeted by vehicles.
*/
private final TargetedPointsSupplier targetedPointsSupplier;
/**
* Indicates whether this component is enabled.
*/
Expand All @@ -48,14 +53,17 @@ public class DefaultRechargePositionSupplier
*
* @param plantModelService The plant model service.
* @param router The router to use.
* @param targetedPointsSupplier Finds all points which are currently targeted by vehicles.
*/
@Inject
public DefaultRechargePositionSupplier(
InternalPlantModelService plantModelService,
Router router
Router router,
TargetedPointsSupplier targetedPointsSupplier
) {
this.plantModelService = requireNonNull(plantModelService, "plantModelService");
this.router = requireNonNull(router, "router");
this.targetedPointsSupplier = requireNonNull(targetedPointsSupplier, "targetedPointsSupplier");
}

@Override
Expand Down Expand Up @@ -93,7 +101,7 @@ public List<DriveOrder.Destination> findRechargeSequence(Vehicle vehicle) {
= findLocationsForOperation(
vehicle.getRechargeOperation(),
vehicle,
router.getTargetedPoints()
targetedPointsSupplier.getTargetedPoints()
);

String assignedRechargeLocationName = vehicle.getProperty(PROPKEY_ASSIGNED_RECHARGE_LOCATION);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ public long getCosts(
}
}

@Deprecated
@Override
public void selectRoute(Vehicle vehicle, List<DriveOrder> driveOrders) {
requireNonNull(vehicle, "vehicle");
Expand All @@ -265,13 +266,15 @@ public void selectRoute(Vehicle vehicle, List<DriveOrder> driveOrders) {
}
}

@Deprecated
@Override
public Map<Vehicle, List<DriveOrder>> getSelectedRoutes() {
synchronized (this) {
return new HashMap<>(routesByVehicle);
}
}

@Deprecated
@Override
public Set<Point> getTargetedPoints() {
synchronized (this) {
Expand Down
Loading

0 comments on commit 5743901

Please sign in to comment.