Skip to content

Lockin#222

Merged
jordens merged 15 commits into
quartiq:masterfrom
matthuszagh:lockin
Jan 13, 2021
Merged

Lockin#222
jordens merged 15 commits into
quartiq:masterfrom
matthuszagh:lockin

Conversation

@matthuszagh

Copy link
Copy Markdown
Contributor

Integrate PLL and integer math to lock-in functionality.

@matthuszagh
matthuszagh requested a review from jordens January 9, 2021 18:51

@jordens jordens left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest you try to actually use the code in the processing ISR in main.rs. That should show whether these wrappers are actually convenient or whether the code would be much shorter and easier to understand if they were inlined.

The moving of the batch and timer constants here doesn't seem right. dsp should be standalone and should not dictate the application. Inlining the code may quickly make the TODO redundant.

Comment thread dsp/src/lib.rs Outdated
Comment thread dsp/src/lockin.rs Outdated
Comment thread dsp/src/lockin.rs Outdated
Comment thread dsp/src/lockin.rs Outdated
Comment thread dsp/src/lockin.rs Outdated
Comment thread dsp/src/lockin.rs Outdated
Comment thread dsp/src/lockin.rs Outdated
@matthuszagh
matthuszagh requested a review from jordens January 13, 2021 04:23

@jordens jordens left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Once TimestampHandler is factored out into dsp (see below), let's move the remaining changes to main.rs into a new PR (e.g. lockin-bin). Then this dsp PR can land right now and we can use the lockin-bin as a trial for the multiple-bin refactoring.

Comment thread src/main.rs Outdated
Comment thread src/main.rs Outdated
Comment thread src/main.rs Outdated
Comment thread src/main.rs Outdated
@matthuszagh
matthuszagh requested a review from jordens January 13, 2021 17:43

@jordens jordens left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

@jordens
jordens merged commit 1d0e1f9 into quartiq:master Jan 13, 2021
@matthuszagh
matthuszagh deleted the lockin branch January 13, 2021 17:49
@matthuszagh matthuszagh mentioned this pull request Jan 13, 2021
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