crotwell / seisFile

A library for reading and writing seismic file formats in java.
GNU Lesser General Public License v3.0
27 stars 20 forks source link

Commands require OK #21

Closed drammelt closed 3 years ago

drammelt commented 3 years ago

Commands such as TIME currently require an OK to be returned to continue otherwise an exception is thrown.

    protected void internalSendCmd(String cmd) throws IOException, SeedlinkException {
        send(cmd);
        String line = readLine();
        if (!line.equals("OK")) {
            throw new SeedlinkException("Command " + cmd + " did not return OK");
        }
    }

The server we are interacting with does not return 'OK' it just returns the data for the command, but does return 'ERROR' if a command fails. If the server is returning 'ERROR' it makes the 'OK' redundant since you could just check if 'ERROR' throw exception.

As per my last bug, this all works fine with the slinktool

From the slinktool changelog

  • Add a few lines to catch a return of 'ERROR\r\n' in the main loop for the response to commands that are unsupported. This is mostly for handling the server response when INFO messages are turned off.
crotwell commented 3 years ago

Is the server you are connecting to public? If so it is probably more efficient if I can test directly.

drammelt commented 3 years ago

All of our sensors are currently private, I have created a private maven repo with a fixed version. However I doubt it is suitable for general release. I have changed the check for OK to throw exception if the line is ERROR, and further in I had to remove the check for the packet header as well as our server just spits out the data without headers (again works fine with slinktool).

If you like I can try and clean it up a bit and submit a pull request?

crotwell commented 3 years ago

I suspect this allowing this may be a bit larger than just checking for ERROR. Basically, if I read the next line to check for OK I might be reading part of a seedlink packet/miniseed record. Currently I assume either an OK or a ERROR line will be sent, and so read until a newline, but that may not exist. And so the read would have to be limited, checked to see if the values look like text vs. seedlink packet/miniseed and then "fixing" or pushing back the partially read seedlink packet/miniseed if it is not text. This is probably doable by looking for the "SL" that starts each seedlink packet.

I do not understand what you mean by "remove the check for the packet header"? If your server is not sending seedlink packets, then is it really seedlink?

Lastly, while it is nice that your servers work with slinktool, that doesn't imply that the server is correctly implementing seedlink. I will try to be accommodating, but I think my code implements the seedlink protocol correctly and so there is a limit to how many changes I am willing to make to support non-standard servers.

Some docs on the protocol are here: https://www.seiscomp.de/doc/apps/seedlink.html

Basically if you can show me that I have not implemented the protocol correctly, I will fix it. If the request is to make a change to support a server that doesn't conform to the protocol as documented above, I might make the change, but will want to make sure it does not have unintended side effects for users connecting to compliant servers.

Lastly, my seedlink code is intended to be used in "multistation" mode, which has some differences from "single station" mode. In particular, in multistation mode data should not start flowing until the client sends "END", by calling endHandshake() in my code, signifying that no further configuraton will be sent. That might explain some of your difficulties.

drammelt commented 3 years ago

I am beginning to suspect you may be correct about the validity of the servers seedlink conformance, I believe the developer who set it up simply made it work with slinktool with no regard to any external compatibility as they never used any standard library I suspect due to the fact they did not understand how to link the library at compile time. We have begun the process of re-writing the firmware for the sensors and will use the iris C seedlink libs which will more than likely eliminate these issues for us.

That documentation is extremely helpful as well I will pass it onto the developer working on the server side of things.

Thank you very much for your help on this issue in any case, I am successfully using seisfile with my custom patches to work with the data from the server so will close the issue.