Skip to content
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

Refactor logic analyzer #167

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

bessman
Copy link
Collaborator

@bessman bessman commented Mar 7, 2024

This is the first part of the major refactorization I'm currently working on. It refactors the logic analyzer instrument, as well as the underlying hardware drivers it depends on, including:

  • Input Capture (IC)
  • Direct Memory Access (DMA)
  • Input Change Notification (CN)
  • Timers (TMR)

This pull request breaks any instrument which depend on an interrupt service routine from any of the above drivers. This is because I'm refactoring the interrupts to be more modular by using callbacks. The broken instruments will be fixed as part of the ongoing refactorization.

The new low-level hardware drivers currently reside in src/registers_ng, to be renamed to src/registers once the refactorization is complete. These drivers are designed to be self-contained, i.e. they have no dependencies on any resources other than themselves. This design aims to solve the current spaghetti-ness of the firmware code, where every module depends on several other modules, often in a transitive or circular manner.

Depends on #165 and #166.

@bessman bessman changed the title Refector logic analyzer Refactor logic analyzer Mar 7, 2024
@CloudyPadmal
Copy link
Collaborator

I was thinking about having a separate development branch. That way, we have a protected master or main branch and once development is tested, we can merge the two. What do you think?

@bessman
Copy link
Collaborator Author

bessman commented Mar 7, 2024

In general, I don't think it's necessary for small projects with few committers to have parallel main and development branches. I prefer to have all development go into main, while tagging periodic releases.

On the other hand, merging known broken code (which this is) into main is nasty. So in this case, it makes sense to keep this work in a separate branch until the refactorization is complete.

@bessman bessman closed this Mar 7, 2024
@bessman bessman deleted the refector/logic_analyzer branch March 7, 2024 18:49
@bessman bessman restored the refector/logic_analyzer branch March 7, 2024 18:50
@bessman bessman reopened this Mar 7, 2024
@bessman bessman force-pushed the refector/logic_analyzer branch 3 times, most recently from 8d93290 to 466d79a Compare March 10, 2024 20:00
@bessman bessman changed the base branch from main to develop March 10, 2024 20:02
@bessman bessman force-pushed the refector/logic_analyzer branch 2 times, most recently from eef9251 to 33448ed Compare March 10, 2024 20:18
@bessman bessman marked this pull request as ready for review March 26, 2024 22:52
@bessman
Copy link
Collaborator Author

bessman commented Mar 26, 2024

Going to merge this soon (into the new and probably temporary 'develop' branch).

Some notes on this change:

  • Some functionality has been removed from the logic analyzer, specifically:
    • The ability to cascade and prescale timers. This feature is useful to look at signals with long time periods between edges. I see this as a niche use case, but the feature could be re-added in the future.
    • The ability to capture such events as 'every fours rising edge' and 'every sixteenth rising edge' and the like. I don't see a use case for this. Available capture events are now 'any edge', 'falling edge', and 'rising edge'.
    • The ability to configure multiple pins as trigger source simultaneously. I don't see a use case for this. At most one pin at a time can now be configured as trigger source.
  • A lot of now-obsolete code hasn't yet been removed. It will be removed at a later stage of the refactorization (in a separate pull request).

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