Skip to content

Logic2 refactoring#20

Open
AnthonZh wants to merge 11 commits intomainfrom
logic2_refactoring
Open

Logic2 refactoring#20
AnthonZh wants to merge 11 commits intomainfrom
logic2_refactoring

Conversation

@AnthonZh
Copy link
Collaborator

Updated revised_scripts/ to use Logic2 software instead of the original Logic1

Changes

  1. Updated requirements.txt to use the logic2-automation python package
  2. Rewrote revised_scripts/logic/logic.py to use Logic2 Software. Note that merging this will make Logic1 no longer work
  3. Updated revised_scripts/Controller.py and revised_scripts/recorder.py to match with the new Logic2 scripts
  4. Updated revised_scripts/hydrophones/hydrophone_array.py to use the new data format which the Logic2 scripts saves the binary files as

Limitations

First, csv files will no longer work as previously. Using data collection current only works with binary files. Also, parser.py may no longer work with the new format in which the data is saved from Logic2.

@AnthonZh AnthonZh requested a review from Saagar-Arya as a code owner March 12, 2026 17:05
@PatrickZheng0 PatrickZheng0 self-requested a review March 14, 2026 03:08
@PatrickZheng0
Copy link
Collaborator

Overall looks solid, not many comments from me since I'm just not familiar with this library/code. For some reason, the overall PR has no files changed. Maybe its bc you haven't yet resolved merge conflicts with main? In any case, I looked through the commits, and other than some not having files changed either, I don't see obvious issues. As long as you tested it, I trust it. Few comments:

A great deal of our data is still in the Logic 1 format, and so I don't think it's a good idea to entirely deprecate it. Can we have a flag to toggle whether we're parsing Logic 1 or Logic 2 data?

Could you briefly describe any testing conducted in the PR description?

@Saagar-Arya do you have comments about the limitations? I'm not familiar with those files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants