Conversation
| // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> | ||
| SDSArchive::SDSArchive() {} | ||
| SDSArchive::SDSArchive() { | ||
| _format = 0; |
There was a problem hiding this comment.
It is more readable to do an inline initialization at the member declaration:
int _format{0};
This also avoid initializing this member twice in different constructors.
| if ( name == "format" ) { | ||
| if ( !Core::fromString(_format, value) || | ||
| _format < 2 || _format > 3 ) | ||
| return false; |
There was a problem hiding this comment.
Logging an error would be nice, wouldn't it?
| bool SDSArchive::setSource(const string &src) { | ||
| if ( src.empty() ) { | ||
| string arcroots; | ||
| size_t pos = src.find('?'); |
There was a problem hiding this comment.
Here we could use Util::Url to parse the given source. This will give you access to URL parameters and do the proper decoding of all components.
| return resolveCha(pathStr, net, sta, loc, cha, requestStartTime, doy, year, first); | ||
| bool res = true; | ||
|
|
||
| if (_format == 0 || _format == 2) |
There was a problem hiding this comment.
Please add spaces within parentheses. Furthermore use curly brackets for conditional blocks even if they are just one-liners.
|
The code is in general looking good. Is this decision to use |
There is a pull request at EarthScope/slarchive#6 Anyway, it's just a proposal. I don't expect that slarchive4 fully replaces slarchive yet, but people who need mseed3 can use it together with seedlink4 and chain4_plugin from https://github.com/andres-h/seedlink4 I've heard that EarthScope is planning to start seedlink4 streaming in few months.
That would probably break too many tools. |
|
@andres-h, are you done with that PR? Do you consider it as final to be merged? |
|
I will make the requested changes. It is not ready yet. Furthermore, I think there may be problems with variable blocksize. I haven't tested it. |
|
I tested this with variable blocksize. It seems to work OK, albeit less efficient (linear search) and I get a warning like this: or more often this: Not sure if anything needs to be done. I created variable blocksize artificially and mseed3 with fixed blocksize (all blocks in file having the same size) works without any problems. |
|
@andres-h Can you quantify "less efficient"? |
|
I think that PR is considered finished, right? In order to get rid of the warnings, we need to disable binary search when searching the first record in case of mseed3 as blocksizes are not fixed.
|
3 participants
Not 100% sure if done correctly, but seems to work. Ideally mseed2 should take precedence if both formats are available.