-
Notifications
You must be signed in to change notification settings - Fork 18
Newer scanner support #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Newer scanner support #41
Conversation
Keeping the same size but with the correct type doesn't seem to work.
033e8c4 to
8dfea83
Compare
This prevents a segfault when filtering 16 to 8 bit data.
sbesson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few inline questions but overall, the changes make sense from a code review perspective.
Does https://github.com/glencoesoftware/isyntax2raw?tab=readme-ov-file#areas-to-improve need to be updated to document the new expectations in terms of support?
Generally, are we close to dropping support for iSyntax SDK 1.x since support for the new scanners including the 16 to 8 bit filter will only be available in the iSyntax SDK 2.x if I understand?
Discussed a bit separately, but that's a little tricky. The changes here are meant to be specific to SDK v2, but we don't have a setup to test SDK v1 (nor are we likely to), so saying for sure that there is no impact on v1 use is difficult. I also wouldn't expect new scanner data to work with v1 at all. In the worst case, anyone who can't upgrade to v2 for whatever reason may need to stick with an older version of isyntax2raw. |
mabruce
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as expected on both an older and newer image, with and without --linear16to8.
This is one approach to fixing #40, but has not yet been tested on real data.This is meant to fix the issue noted in #40, and subsequent issues on the same dataset.
With dateutil/dateutil#1252 still open, the
%Y%m%d%H%M%S.%f%zformat is not handled automatically. Separate cases for%Y%m%d%H%M%S.%f%zand%Y%m%d%H%M%S.%fappear to be necessary, as if I try a little test like this (using timestamps from #40 and #39 (comment)):