Skip to content

Conversation

@AaronJackson
Copy link
Contributor

No description provided.

@AaronJackson
Copy link
Contributor Author

Decided to tweak this a bit after using it today (8051 cpu adventures 😱)

When viewing the vcd file it was a bit annoying to see the idle time before it would trigger, so the changes raises an event to inform the interact function of triggering, so an offset can be applied.

i'm sure there will be comments, this is my first time trying to do anything useful in amaranth, so i'll save a rebase until later...

thanks!


parser.add_argument(
"--trigger", metavar="PATTERN", type=bit_pattern, default=None,
help="optional one-shot trigger pattern. e.g. 1xx0"
Copy link
Contributor

Choose a reason for hiding this comment

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

From a user perspective — how does this work? Which input is this pattern applied to, and how does time factor into it? In other words, what kind of real-world quantity corresponds to one bit in the pattern specification? (I think this worth specifying in the help output or manual)

Based on the code, it seems to be a spacial rather than temporal pattern, i.e. 1xx0 means pins 0 must be 1, and pin 3 must be 0 for the pattern to trigger, but I might be misreading it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your guess is correct, although when I opened this PR none of the applets had help pages in the docs. I'd be happy to write some if Catherine is happy with the changes.

I'm not a huge fan of the argument name trigger for the reasons you stated. I'm sure there's a better approach to the argument names which will accommodate different types of trigger, but I'm not sure what it is currently. Maybe parallel-trigger

Copy link
Member

Choose a reason for hiding this comment

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

Let's discuss a design for a better LA than what we currently have on the channel.

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.

3 participants