Merge pull request #1027 from SpiNNakerManchester/bmp_sanity

Bmp sanity

SpiNNaker-allocserv/src/main/java/uk/ac/manchester/spinnaker/alloc/bmp/BMPController.java

@@ -67,9 +67,9 @@
 import uk.ac.manchester.spinnaker.alloc.db.Row;
 import uk.ac.manchester.spinnaker.alloc.model.Direction;
 import uk.ac.manchester.spinnaker.alloc.model.JobState;
-import uk.ac.manchester.spinnaker.machine.HasCoreLocation;
 import uk.ac.manchester.spinnaker.machine.board.BMPBoard;
 import uk.ac.manchester.spinnaker.machine.board.BMPCoords;
+import uk.ac.manchester.spinnaker.machine.board.HasBMPLocation;
 import uk.ac.manchester.spinnaker.messages.model.ADCInfo;
 import uk.ac.manchester.spinnaker.messages.model.Blacklist;
 import uk.ac.manchester.spinnaker.transceiver.ProcessException;
@@ -156,6 +156,7 @@ private void init() {
 		var sched = new ThreadPoolTaskScheduler();
 		scheduler = sched;
 		sched.setThreadGroupName("BMP");
+		sched.initialize();
 
 		controllerFactory = controllerFactoryBean::getObject;
 		allocator.setBMPController(this);
@@ -292,7 +293,7 @@ final boolean bmpAction(ThrowingAction body,
 				exn = e;
 				log.error("Requests failed on BMP {}", bmpId, e);
 			} catch (PermanentProcessException e) {
-				log.error("BMP {} on {} is unreachable", e.core, bmpId, e);
+				log.error("BMP {} on {} is unreachable", e.source, bmpId, e);
 				onServiceRemove.accept(e);
 				exn = e;
 			} catch (CallerProcessException e) {
@@ -643,7 +644,7 @@ public String toString() {
 		private void badBoard(ProcessException failure) {
 			try (var c = getConnection()) {
 				c.transaction(() -> {
-					getBoardId(failure.core).ifPresent(boardId -> {
+					getBoardId(failure.source).ifPresent(boardId -> {
 						// Mark the board as dead right now
 						markBoardAsDead(c, boardId, REPORT_MSG + failure);
 						// Add a report if we can
@@ -661,8 +662,8 @@ private void badBoard(ProcessException failure) {
 		 *            The board address.
 		 * @return The ID, if one can be found.
 		 */
-		private Optional<Integer> getBoardId(HasCoreLocation addr) {
-			return Optional.ofNullable(boardToId.get(addr.getP()));
+		private Optional<Integer> getBoardId(HasBMPLocation addr) {
+			return Optional.ofNullable(boardToId.get(addr.getBoard()));
 		}
 
 		private Integer getBoardId(BMPBoard board) {

SpiNNaker-allocserv/src/main/java/uk/ac/manchester/spinnaker/alloc/bmp/FirmwareLoader.java

@@ -398,13 +398,13 @@ private void updateFlashData(ByteBuffer data)
 	private void logBMPVersion()
 			throws ProcessException, IOException, InterruptedException {
 		var info = txrx.readBMPVersion(board);
-		// TODO validate which field is which; some of these seem... unlikely
 		log.info("BMP INFO:       {}",
 				format("%s %s at %s:%s (built %s) [C=%s, F=%s, B=%s]",
 						info.name, info.versionNumber, info.hardware,
-						info.core.getP(), Instant.ofEpochSecond(info.buildDate),
-						info.physicalCPUID, info.core.getY(),
-						info.core.getX()));
+						info.physicalCPUID,
+						Instant.ofEpochSecond(info.buildDate),
+						info.location.getCabinet(), info.location.getFrame(),
+						info.location.getBoard()));
 	}
 
 	/**

SpiNNaker-comms/src/main/java/uk/ac/manchester/spinnaker/connections/UDPConnection.java

@@ -49,8 +49,8 @@
 import com.google.errorprone.annotations.OverridingMethodsMustInvokeSuper;
 
 import uk.ac.manchester.spinnaker.connections.model.Connection;
-import uk.ac.manchester.spinnaker.machine.CoreLocation;
 import uk.ac.manchester.spinnaker.messages.sdp.SDPHeader;
+import uk.ac.manchester.spinnaker.messages.sdp.SDPLocation;
 import uk.ac.manchester.spinnaker.messages.sdp.SDPMessage;
 
 /**
@@ -697,7 +697,7 @@ public final void sendPortTriggerMessage(InetAddress host)
 		 * REPLY_NOT_EXPECTED flag is set (see scamp-3.c line 728 and 625-644)
 		 */
 		var triggerMessage = new SDPMessage(new SDPHeader(REPLY_NOT_EXPECTED,
-				new CoreLocation(0, 0, 0), RUNNING_COMMAND_SDP_PORT));
+				new SDPLocation(0, 0, 0), RUNNING_COMMAND_SDP_PORT));
 		sendTo(triggerMessage.getMessageData(null), host, SCP_SCAMP_PORT);
 	}
 

SpiNNaker-comms/src/main/java/uk/ac/manchester/spinnaker/messages/bmp/BMPRequest.java

@@ -31,6 +31,7 @@
 import uk.ac.manchester.spinnaker.messages.scp.SCPRequest;
 import uk.ac.manchester.spinnaker.messages.scp.SCPResponse;
 import uk.ac.manchester.spinnaker.messages.sdp.SDPHeader;
+import uk.ac.manchester.spinnaker.messages.sdp.SDPLocation;
 import uk.ac.manchester.spinnaker.utils.UsedInJavadocOnly;
 
 /**
@@ -43,7 +44,7 @@
 public abstract class BMPRequest<T extends BMPRequest.BMPResponse>
 		extends SCPRequest<T> {
 	private static SDPHeader header(int board) {
-		return new SDPHeader(REPLY_EXPECTED, new BMPLocation(board),
+		return new SDPHeader(REPLY_EXPECTED, new SDPLocation(0, 0, board),
 				DEFAULT_PORT);
 	}
 

SpiNNaker-comms/src/main/java/uk/ac/manchester/spinnaker/messages/model/VersionInfo.java

@@ -26,9 +26,7 @@
 import java.nio.ByteBuffer;
 import java.util.regex.Pattern;
 
-import uk.ac.manchester.spinnaker.machine.CoreLocation;
-import uk.ac.manchester.spinnaker.machine.HasCoreLocation;
-import uk.ac.manchester.spinnaker.messages.bmp.BMPLocation;
+import uk.ac.manchester.spinnaker.messages.sdp.SDPLocation;
 
 /**
  * Decodes SC&MP/SARK version information as returned by the SVER command.
@@ -57,7 +55,7 @@ public final class VersionInfo {
 	public final String versionString;
 
 	/** The location of the core where the information was obtained. */
-	public final HasCoreLocation core;
+	public final SDPLocation location;
 
 	private static final Pattern VERSION_RE = Pattern
 			.compile("(?<major>\\d+)\\.(?<minor>\\d+)\\.(?<revision>\\d+)");
@@ -100,11 +98,7 @@ public VersionInfo(ByteBuffer buffer, boolean isBMP) {
 		physicalCPUID = toUnsignedInt(buffer.get());
 		int y = toUnsignedInt(buffer.get());
 		int x = toUnsignedInt(buffer.get());
-		if (isBMP) {
-			core = new BMPLocation(x, y, p);
-		} else {
-			core = new CoreLocation(x, y, p);
-		}
+		location = new SDPLocation(x, y, p);
 		buffer.getShort(); // Ignore 2 bytes
 		int vn = toUnsignedInt(buffer.getShort());
 		buildDate = buffer.getInt();
@@ -144,7 +138,7 @@ public VersionInfo(ByteBuffer buffer, boolean isBMP) {
 
 	@Override
 	public String toString() {
-		return "VersionInfo(" + core + " (phys:" + physicalCPUID
+		return "VersionInfo(" + location + " (phys:" + physicalCPUID
 				+ "), version: " + versionNumber + ", " + name + "/" + hardware
 				+ ", " + ofInstant(ofEpochSecond(buildDate, 0), UTC)
 						.format(ISO_INSTANT)

SpiNNaker-comms/src/main/java/uk/ac/manchester/spinnaker/messages/scp/FECRequest.java

@@ -22,6 +22,7 @@
 
 import uk.ac.manchester.spinnaker.machine.HasCoreLocation;
 import uk.ac.manchester.spinnaker.messages.sdp.SDPHeader;
+import uk.ac.manchester.spinnaker.messages.sdp.SDPLocation;
 
 /**
  * A command message to a core using FrontEndCommon lib's simulation management
@@ -79,7 +80,7 @@ public abstract class FECRequest<T extends SCPResponse> extends SCPRequest<T> {
 	private static SDPHeader header(HasCoreLocation core,
 			boolean replyExpected) {
 		return new SDPHeader(
-				replyExpected ? REPLY_EXPECTED : REPLY_NOT_EXPECTED, core,
-				RUNNING_COMMAND_SDP_PORT);
+				replyExpected ? REPLY_EXPECTED : REPLY_NOT_EXPECTED,
+				new SDPLocation(core), RUNNING_COMMAND_SDP_PORT);
 	}
 }

SpiNNaker-comms/src/main/java/uk/ac/manchester/spinnaker/messages/scp/ReinjectorRequest.java

@@ -22,6 +22,7 @@
 
 import uk.ac.manchester.spinnaker.machine.HasCoreLocation;
 import uk.ac.manchester.spinnaker.messages.sdp.SDPHeader;
+import uk.ac.manchester.spinnaker.messages.sdp.SDPLocation;
 
 /**
  * A command message to an extra monitor control core to manipulate the packet
@@ -86,7 +87,7 @@ public abstract class ReinjectorRequest<T extends CheckOKResponse>
 	 * @return The SDP header.
 	 */
 	private static SDPHeader header(HasCoreLocation core) {
-		return new SDPHeader(REPLY_EXPECTED, core,
+		return new SDPHeader(REPLY_EXPECTED, new SDPLocation(core),
 				EXTRA_MONITOR_CORE_REINJECTION);
 	}
 }

SpiNNaker-comms/src/main/java/uk/ac/manchester/spinnaker/messages/scp/RouterTableRequest.java

@@ -25,6 +25,7 @@
 import uk.ac.manchester.spinnaker.machine.HasCoreLocation;
 import uk.ac.manchester.spinnaker.messages.model.UnexpectedResponseCodeException;
 import uk.ac.manchester.spinnaker.messages.sdp.SDPHeader;
+import uk.ac.manchester.spinnaker.messages.sdp.SDPLocation;
 
 /**
  * A command message to an extra monitor control core to manipulate the router
@@ -70,6 +71,7 @@ public final EmptyResponse getSCPResponse(ByteBuffer buffer)
 	 * @return The SDP header.
 	 */
 	static final SDPHeader header(HasCoreLocation core) {
-		return new SDPHeader(REPLY_EXPECTED, core, GATHERER_DATA_SPEED_UP);
+		return new SDPHeader(REPLY_EXPECTED, new SDPLocation(core),
+				GATHERER_DATA_SPEED_UP);
 	}
 }

SpiNNaker-comms/src/main/java/uk/ac/manchester/spinnaker/messages/scp/SCPRequest.java

@@ -25,6 +25,7 @@
 import uk.ac.manchester.spinnaker.machine.CoreLocation;
 import uk.ac.manchester.spinnaker.machine.HasCoreLocation;
 import uk.ac.manchester.spinnaker.messages.sdp.SDPHeader;
+import uk.ac.manchester.spinnaker.messages.sdp.SDPLocation;
 import uk.ac.manchester.spinnaker.messages.sdp.SpinnakerRequest;
 
 /**
@@ -82,7 +83,8 @@ public abstract class SCPRequest<T extends SCPResponse>
 	 * @return The SDP header.
 	 */
 	private static SDPHeader header(HasCoreLocation core) {
-		return new SDPHeader(REPLY_EXPECTED, core, DEFAULT_PORT);
+		return new SDPHeader(REPLY_EXPECTED, new SDPLocation(core),
+				DEFAULT_PORT);
 	}
 
 	/**

SpiNNaker-comms/src/main/java/uk/ac/manchester/spinnaker/messages/sdp/SDPHeader.java

@@ -16,20 +16,15 @@
 package uk.ac.manchester.spinnaker.messages.sdp;
 
 import static java.lang.Byte.toUnsignedInt;
-import static uk.ac.manchester.spinnaker.machine.MachineDefaults.MAX_NUM_CORES;
-import static uk.ac.manchester.spinnaker.machine.MachineDefaults.validateChipLocation;
 import static uk.ac.manchester.spinnaker.utils.CollectionUtils.makeEnumBackingMap;
 
 import java.nio.ByteBuffer;
 import java.util.Map;
 
 import javax.validation.Valid;
-import javax.validation.constraints.PositiveOrZero;
 
 import uk.ac.manchester.spinnaker.machine.CoreLocation;
-import uk.ac.manchester.spinnaker.machine.HasCoreLocation;
 import uk.ac.manchester.spinnaker.messages.SerializableMessage;
-import uk.ac.manchester.spinnaker.messages.bmp.BMPLocation;
 
 /**
  * Represents the header of an SDP message.
@@ -48,12 +43,12 @@ public final class SDPHeader implements SerializableMessage {
 	private static final int PORT_MASK = (1 << PORT_BITS) - 1;
 
 	@Valid
-	private HasCoreLocation destination;
+	private SDPLocation destination;
 
 	private int destinationPort;
 
 	@Valid
-	private HasCoreLocation source;
+	private SDPLocation source;
 
 	private int sourcePort;
 
@@ -85,7 +80,7 @@ public SDPHeader() {
 	 * @throws IllegalArgumentException
 	 *             if a bad SDP port is given
 	 */
-	public SDPHeader(Flag flags, HasCoreLocation destination,
+	public SDPHeader(Flag flags, SDPLocation destination,
 			int destinationPort) {
 		this.flags = flags;
 		this.destination = destination;
@@ -109,7 +104,7 @@ public SDPHeader(Flag flags, HasCoreLocation destination,
 	 *            this is <b>not</b> a UDP port! Those are associated with a
 	 *            connection, not a message.
 	 */
-	public SDPHeader(Flag flags, HasCoreLocation destination,
+	public SDPHeader(Flag flags, SDPLocation destination,
 			SDPPort destinationPort) {
 		this.flags = flags;
 		this.destination = destination;
@@ -137,52 +132,8 @@ public SDPHeader(ByteBuffer buffer, boolean isBMP) {
 		int scx = toUnsignedInt(buffer.get());
 		destinationPort = (dpc >> CPU_ADDR_BITS) & PORT_MASK;
 		sourcePort = (spc >> CPU_ADDR_BITS) & PORT_MASK;
-		if (isBMP) {
-			destination = new BMPLocation(dcx, dcy, dpc & CPU_MASK);
-			source = new BMPLocation(scx, scy, spc & CPU_MASK);
-		} else {
-			destination = allocCoreLocation(dcx, dcy, dpc & CPU_MASK);
-			source = allocCoreLocation(scx, scy, spc & CPU_MASK);
-		}
-	}
-
-	private HasCoreLocation allocCoreLocation(int x, int y, int p) {
-		if (p >= 0 && p < MAX_NUM_CORES) {
-			return new CoreLocation(x, y, p);
-		}
-		validateChipLocation(x, y);
-		return new HasCoreLocation() {
-			@Override
-			public int getX() {
-				return x;
-			}
-
-			@Override
-			public int getY() {
-				return y;
-			}
-
-			@Override
-			@PositiveOrZero
-			public int getP() {
-				return p;
-			}
-
-			@Override
-			public int hashCode() {
-				throw new UnsupportedOperationException(
-						"this object may not be used as a key");
-			}
-
-			@Override
-			public boolean equals(Object other) {
-				if (!(other instanceof HasCoreLocation)) {
-					return false;
-				}
-				var c = (HasCoreLocation) other;
-				return x == c.getX() && y == c.getY() && p == c.getP();
-			}
-		};
+		destination = new SDPLocation(dcx, dcy, dpc & CPU_MASK);
+		source = new SDPLocation(scx, scy, spc & CPU_MASK);
 	}
 
 	@Override
@@ -196,18 +147,18 @@ public void addToBuffer(ByteBuffer buffer) {
 		buffer.put((byte) tag);
 		buffer.put((byte) dpc);
 		buffer.put((byte) spc);
-		buffer.put((byte) destination.getY());
-		buffer.put((byte) destination.getX());
-		buffer.put((byte) source.getY());
-		buffer.put((byte) source.getX());
+		buffer.put((byte) destination.minor);
+		buffer.put((byte) destination.major);
+		buffer.put((byte) source.minor);
+		buffer.put((byte) source.major);
 	}
 
 	/**
 	 * @return What "chip" is the message going to. Note that there is a special
 	 *         address for addresses off the machine; that address is not
 	 *         normally representable in a {@link CoreLocation}.
 	 */
-	public HasCoreLocation getDestination() {
+	public SDPLocation getDestination() {
 		return destination;
 	}
 
@@ -217,7 +168,7 @@ public HasCoreLocation getDestination() {
 	 *            special address for addresses off the machine; that address
 	 *            is not normally representable in a {@link CoreLocation}.
 	 */
-	public void setDestination(HasCoreLocation destination) {
+	public void setDestination(SDPLocation destination) {
 		this.destination = destination;
 	}
 
@@ -252,7 +203,7 @@ public void setDestinationPort(int port) {
 	}
 
 	/** @return What "chip" originated the message. */
-	public HasCoreLocation getSource() {
+	public SDPLocation getSource() {
 		return source;
 	}
 
@@ -262,7 +213,7 @@ public HasCoreLocation getSource() {
 	 *            special address for origination off the machine; that address
 	 *            is not normally representable in a {@link CoreLocation}.
 	 */
-	public void setSource(HasCoreLocation source) {
+	public void setSource(SDPLocation source) {
 		this.source = source;
 	}