Skip to content

Commit

Permalink
[#175] fix ZscriptBasicNode default error-handler functions separated
Browse files Browse the repository at this point in the history
  • Loading branch information
susanw1 committed Jan 16, 2025
1 parent 7032822 commit b691a35
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,12 @@ private ResponseExecutionPath(@Nullable ResponseElement firstResponse) {
this.firstResponse = firstResponse;
}

@Nullable
public ResponseElement getFirstResponse() {
return firstResponse;
}

@Nonnull
public List<ResponseElement> getResponses() {
List<ResponseElement> resps = new ArrayList<>();
for (ResponseElement r = firstResponse; r != null; r = r.getNext()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ public boolean send(CommandSequence seq, boolean ignoreLength, long timeout, Tim
}

public boolean send(CommandExecutionPath path, boolean ignoreLength, long timeout, TimeUnit unit) {
return send(new BufferElement(path, System.nanoTime() + unit.toNanos(timeout)), ignoreLength);
final CommandSequence seq = CommandSequence.from(path, echo.getEcho(), supports32Locks, lockConditions);
return send(new BufferElement(seq, System.nanoTime() + unit.toNanos(timeout)), ignoreLength);
}

public boolean hasNonAddressedInBuffer() {
Expand All @@ -177,7 +178,7 @@ public void setBufferSize(int bufferSize) {
this.bufferSize = bufferSize;
}

class BufferElement {
static class BufferElement {
private final AddressedCommand cmd;
private final boolean sameLayer;
private final boolean hadEchoBefore;
Expand All @@ -192,15 +193,6 @@ class BufferElement {
this.nanoTimeTimeout = nanoTimeTimeout;
}

BufferElement(CommandExecutionPath cmd, long nanoTimeTimeout) {
CommandSequence seq = CommandSequence.from(cmd, echo.getEcho(), supports32Locks, lockConditions);
this.cmd = new AddressedCommand(seq);
this.sameLayer = true;
this.hadEchoBefore = false;
this.length = seq.getBufferLength();
this.nanoTimeTimeout = nanoTimeTimeout;
}

BufferElement(AddressedCommand cmd, long nanoTimeTimeout) {
this.cmd = cmd;
this.sameLayer = !cmd.hasAddressLayer();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package net.zscript.javaclient.devicenodes;

import javax.annotation.Nullable;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -10,6 +11,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import net.zscript.javaclient.ZscriptClientException;
import net.zscript.javaclient.addressing.AddressedCommand;
import net.zscript.javaclient.addressing.AddressedResponse;
import net.zscript.javaclient.addressing.ZscriptAddress;
Expand All @@ -30,30 +32,14 @@ class ZscriptBasicNode implements ZscriptNode {
private final ConnectionBuffer connectionBuffer;
private final Connection parentConnection;

private QueuingStrategy strategy = new StandardQueuingStrategy(1000, TimeUnit.SECONDS); // should be enough for almost all cases

private BiConsumer<AddressedCommand, AddressedResponse> badCommandResponseMatchHandler = (cmd, resp) -> {
LOG.error("Command and response do not match: {} ; {}", cmd.getContent().asStringUtf8(), resp.getResponseSequence().asStringUtf8());
};

private Consumer<AddressedResponse> unknownNotificationHandler = resp -> {
LOG.warn("Unknown notification received: {}", resp.getResponseSequence().asStringUtf8());
};

private Consumer<AddressedResponse> unknownResponseHandler = resp -> {
throw new IllegalStateException("Unknown response received: " + resp.getResponseSequence().asStringUtf8());
};

private Consumer<Exception> callbackExceptionHandler = e -> {
LOG.error("Exception caught from callback: ", e);
};
private final EchoAssigner echoSystem;

private final Map<Integer, Consumer<ResponseSequence>> notificationHandlers = new HashMap<>();

private final Map<CommandExecutionPath, Consumer<ResponseExecutionPath>> pathCallbacks = new HashMap<>();
private final Map<CommandSequence, Consumer<ResponseSequence>> fullSequenceCallbacks = new HashMap<>();

private final EchoAssigner echoSystem;
private QueuingStrategy strategy = new StandardQueuingStrategy(1000, TimeUnit.SECONDS); // should be enough for almost all cases

ZscriptBasicNode(ZscriptCallbackThreadpool callbackPool, Connection parentConnection, int bufferSize) {
this(callbackPool, parentConnection, bufferSize, 100, TimeUnit.MILLISECONDS);
Expand All @@ -63,9 +49,11 @@ class ZscriptBasicNode implements ZscriptNode {
this.callbackPool = callbackPool;
this.addressingSystem = new AddressingSystem(this);
this.parentConnection = parentConnection;

this.echoSystem = new EchoAssigner(unit.toNanos(minSegmentChangeTime));
this.connectionBuffer = new ConnectionBuffer(parentConnection, echoSystem, bufferSize);
this.strategy.setBuffer(connectionBuffer);

parentConnection.onReceive(resp -> {
try {
if (resp.hasAddress()) {
Expand All @@ -81,23 +69,6 @@ class ZscriptBasicNode implements ZscriptNode {
});
}

public void setUnknownResponseHandler(Consumer<AddressedResponse> unknownResponseHandler) {
this.unknownResponseHandler = unknownResponseHandler;
}

public void setUnknownNotificationHandler(Consumer<AddressedResponse> unknownNotificationHandler) {
this.unknownNotificationHandler = unknownNotificationHandler;
}

public void setBadCommandResponseMatchHandler(
BiConsumer<AddressedCommand, AddressedResponse> badCommandResponseMatchHandler) {
this.badCommandResponseMatchHandler = badCommandResponseMatchHandler;
}

public void setCallbackExceptionHandler(Consumer<Exception> callbackExceptionHandler) {
this.callbackExceptionHandler = callbackExceptionHandler;
}

public void setStrategy(QueuingStrategy strategy) {
this.strategy = strategy;
this.strategy.setBuffer(connectionBuffer);
Expand Down Expand Up @@ -141,6 +112,7 @@ public void checkTimeouts() {

private void response(AddressedResponse resp) {
if (resp.getResponseSequence().getResponseFieldValue() != 0) {
// it's a notification
Consumer<ResponseSequence> handler = notificationHandlers.get(resp.getResponseSequence().getResponseFieldValue());
if (handler != null) {
callbackPool.sendCallback(handler, resp.getResponseSequence(), callbackExceptionHandler);
Expand Down Expand Up @@ -207,4 +179,64 @@ public void removeNotificationHandler(int notification) {
public void setBufferSize(int bufferSize) {
connectionBuffer.setBufferSize(bufferSize);
}

private BiConsumer<AddressedCommand, AddressedResponse> badCommandResponseMatchHandler = ZscriptBasicNode::defaultBadCommandResponseMatchHandler;
private Consumer<AddressedResponse> unknownResponseHandler = ZscriptBasicNode::defaultUnknownResponseHandler;
private Consumer<AddressedResponse> unknownNotificationHandler = ZscriptBasicNode::defaultUnknownNotificationHandler;
private Consumer<Exception> callbackExceptionHandler = ZscriptBasicNode::defaultCallbackExceptionHandler;

private static void defaultBadCommandResponseMatchHandler(AddressedCommand cmd, AddressedResponse resp) {
LOG.error("Command and response do not match: {} ; {}", cmd.getContent().asStringUtf8(), resp.getResponseSequence().asStringUtf8());
}

private static void defaultUnknownResponseHandler(AddressedResponse resp) {
throw new ZscriptClientException("Unknown response received: " + resp.getResponseSequence().asStringUtf8());
}

private static void defaultUnknownNotificationHandler(AddressedResponse resp) {
LOG.warn("Unknown notification received: {}", resp.getResponseSequence().asStringUtf8());
}

private static void defaultCallbackExceptionHandler(Exception e) {
LOG.error("Exception caught from callback: ", e);
}

/**
* Sets the handler for when a response is received for a command, but where its response sequence doesn't match its corresponding command sequence. This would imply something
* bad has happened in the Zscript processing on the target device - sounds like a bug!
*
* @param badCommandResponseMatchHandler the new handler, or null to restore the default handler
*/
public void setBadCommandResponseMatchHandler(@Nullable BiConsumer<AddressedCommand, AddressedResponse> badCommandResponseMatchHandler) {
this.badCommandResponseMatchHandler = badCommandResponseMatchHandler != null ? badCommandResponseMatchHandler : ZscriptBasicNode::defaultBadCommandResponseMatchHandler;
}

/**
* Sets the handler for when a response is received that cannot be related to a known sent command. This would normally mean something weird has happened - it probably
* shouldn't happen.
*
* @param unknownResponseHandler the new handler, or null to restore the default handler
*/
public void setUnknownResponseHandler(@Nullable Consumer<AddressedResponse> unknownResponseHandler) {
this.unknownResponseHandler = unknownResponseHandler != null ? unknownResponseHandler : ZscriptBasicNode::defaultUnknownResponseHandler;
}

/**
* Sets the handler for when a notification is received for which no listener is registered. This might be unexpected and worth knowing about, but may easily be ignored.
*
* @param unknownNotificationHandler the new handler, or null to restore the default handler
*/
public void setUnknownNotificationHandler(@Nullable Consumer<AddressedResponse> unknownNotificationHandler) {
this.unknownNotificationHandler = unknownNotificationHandler != null ? unknownNotificationHandler : ZscriptBasicNode::defaultUnknownNotificationHandler;
}

/**
* Sets the handler for when a response is received for a command, but where its response sequence doesn't match its corresponding command sequence. This would imply something
* bad has happened in the Zscript processing on the target device - sounds like a bug!
*
* @param callbackExceptionHandler the new handler, or null to restore the default handler
*/
public void setCallbackExceptionHandler(@Nullable Consumer<Exception> callbackExceptionHandler) {
this.callbackExceptionHandler = callbackExceptionHandler != null ? callbackExceptionHandler : ZscriptBasicNode::defaultCallbackExceptionHandler;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
public interface ZscriptExpression {
/**
* Accesses all the numeric fields in indeterminate order.
* Accesses all the numeric fields in indeterminate order (not big-fields - use {@link #getBigFieldAsByteString()}).
*
* @return stream of numeric fields
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ public void shouldGetFields() {
assertThat(zscriptExpr.hasBigField()).isFalse();
}

@Test
public void shouldGetAllFields() {
tokenize("Z12 A34 BCffff +12\n");
assertThat(zscriptExpr.fields()).hasSize(4)
.extracting(f -> (char) f.getKey()).containsOnly('A', 'B', 'C', 'Z');

}

@Test
public void shouldGetBigFields() {
tokenize("Z12 A34 BCffff \"Hello\" \n");
Expand Down

0 comments on commit b691a35

Please sign in to comment.