Add Find N'th Pattern component #187
Conversation
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Signed-off-by: laeticiachee <[email protected]>
2. Fix test inputs Signed-off-by: Ramli, Nurul Izziany <[email protected]>
2. Add error signal when pattern is not found Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
doc/components/find.md
Outdated
| To find the nth pattern occurrence just pass in `bus` along with Logic pattern `pattern` passed in as an argument. Note that the `pattern` needs to be smaller than `bus`. | ||
|
|
||
| ### Find Nth Pattern from start | ||
| To find the nth pattern occurrence just pass in `bus` along with Logic pattern `pattern` passed in as an argument. Note that the `pattern` needs to be smaller than `bus`. |
There was a problem hiding this comment.
while the doc comment above in this file is already repetitive and I know you're following the example, we can do a little better: let's not copy paste the same sentence in each section here, just put the relevant new documentation required to use the component
There was a problem hiding this comment.
Will create a separate doc in find_pattern.md as the component description will be a bit different than as describe for Find component in find.md
doc/components/find.md
Outdated
| @@ -36,3 +36,14 @@ To find the nth occurrence just pass in `bus` along with Logic value `n` passed | |||
| ### Find Nth Zero | |||
|
|
|||
| To find the nth occurrence just pass in `bus` along with Logic value `n` passed in as an argument. In addition to finding occurrence of Zero (`0`), set the argument `countOne` to `false`. | |||
|
|
|||
| ## Find Nth Pattern | |||
| To find the nth pattern occurrence just pass in `bus` along with Logic pattern `pattern` passed in as an argument. Note that the `pattern` needs to be smaller than `bus`. | |||
There was a problem hiding this comment.
it would be good to mention the name of the component in here FindPattern
lib/src/find_pattern.dart
Outdated
| Logic get index => output('index'); | ||
|
|
||
| /// [error] is a getter for error in FindPattern | ||
| /// When your pattern is not found it will result in error `1` |
There was a problem hiding this comment.
It would be good to mention in the doc comment here that this signal only exists when generateError is true
lib/src/find_pattern.dart
Outdated
| } | ||
|
|
||
| // Initialize counter pattern occurrence to 0 | ||
| Logic count = Const(0, width: bus.width); |
There was a problem hiding this comment.
nit: technically, the width of this count only needs to be ~log2ceil of the bus width (to represent an index in bus)
lib/src/find_pattern.dart
Outdated
| final valCheck = busVal.eq(pattern); | ||
|
|
||
| // Check if pattern matches, count if found | ||
| count += ((valCheck.value.toInt() == 1) ? 1 : 0); |
There was a problem hiding this comment.
I think you mean count += valCheck directly -- it's already a 1-bit signal and you want to assign it synthesizably, not based on .toInt()
lib/src/find_pattern.dart
Outdated
| if (generateError) { | ||
| // If pattern is not found, return error | ||
| final isError = | ||
| (count.value.toInt() < nVal.value.toInt() || count.value.toInt() == 0) |
There was a problem hiding this comment.
Again, operate on the signals themselves. Something like isError = count.lt(nVal) | count.eq(0)
| import 'package:rohd_hcl/rohd_hcl.dart'; | ||
| import 'package:test/test.dart'; | ||
|
|
||
| void main() { |
There was a problem hiding this comment.
suggestion: try writing at least one test where you do multiple different values on the bus, pattern, and n for the same hardware. this will help test against the type of .toInt() bugs that you have in your implementation
There was a problem hiding this comment.
suggestion: try writing at least one test where you do multiple different values on the bus, pattern, and
nfor the same hardware. this will help test against the type of.toInt()bugs that you have in your implementation
Can you describe more what do you mean by multiple values for the same hardware here?
| /// Outputs pin `index` contains position. | ||
| class FindPattern extends Module { | ||
| /// [index] is a getter for output of FindPattern | ||
| Logic get index => output('index'); |
There was a problem hiding this comment.
It would be good to clarify in the doc comment what the index means exactly. For example, if you have a pattern 01, is the index the position of the 0 or the 1? Does it depend on the direction of start?
| /// Outputs pin `index` contains position. | ||
| class FindPattern extends Module { | ||
| /// [index] is a getter for output of FindPattern | ||
| Logic get index => output('index'); |
There was a problem hiding this comment.
What does the index output when there is an error? Would be good to document (even if you claim "undefined behavior")
lib/src/find_pattern.dart
Outdated
| /// | ||
| /// Outputs pin `index` contains the position. Position starts from `0` based. | ||
| FindPattern(Logic bus, Logic pattern, | ||
| {bool start = true, Logic? n, this.generateError = false}) |
There was a problem hiding this comment.
suggestion: a name of fromStart or something might be more clear
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Signed-off-by: Ramli, Nurul Izziany <[email protected]>
Description & Motivation
Similar to Find N'th bit from start/end, but FindPattern find an entire multi-bit pattern from start/end and returns it index
Related Issue(s)
#169
Testing
Test is included in test/find_pattern_test.dart
Backwards-compatibility
No
Documentation
Documentation added in doc/components/find.md