wavedrom
wavedrom copied to clipboard
More sub-cycle tests
h<.>l
{signal: [
{name: 'pulse', wave: '0.h<.>l.'}
]}
expected: 1.5 cycle active-high pulse
<h>.l
{signal: [
{name: 'pulse', wave: '0.<h>.l.'}
]}
expected: 1.5 cycle active-high pulse
.<h..>l
{signal: [
{name: 'pulse', wave: '0.<h..>l.'}
]}
expected: 1.5 cycle active-high pulse
I have a few more as I worked a lot on fixing this in wavedrompy. They are from my regressions (https://github.com/wallento/wavedrompy/blob/master/test/regressions.py#L85)
<10>10
{ "signal": [
{ "name": "sig", "wave": "<10>10" }
]
}
Expected:
<x0>1x
{ "signal": [
{ "name": "sig", "wave": "<x0>1x" }
]
}
Expected:
<01>
{ "signal": [
{ "name": "sig", "wave": "<01>" }
]
}
Expected:
x.<01...0>x
{ "signal": [
{ "name": "sig", "wave": "x.<01...0>x" }
]
}
Expected:
<=|>.x
{ "signal": [
{ "name": "sig", "wave": "<=|>.x" }
]
}
Expected:
x<=|>.x
{ "signal": [
{ "name": "sig", "wave": "x<=|>.x" }
]
}
Expected:
x<.|>0x
{ "signal": [
{ "name": "sig", "wave": "x<.|>0x" }
]
}
Expected:
Aliaksei,
I have a patch that resolves all of the above rendering issues, as well as the signals from Cliff's e-mail report in Issue #163 / quirks in the closing remarks in the original enhancement request #148 .
The current rendering for the full test set is in this shortened wavedrom editor URL : https://tinyurl.com/y45gajkr ; Please rotate layout to compare with the rendering based on my patch - picture attached:
I am not really familiar with Git stuff yet, so I am just attaching the single file that I modified as part of my patch as a ZIP archive.
parse-wave-lane-ganeshts-patch-09102019.zip
One aspect that I could not fix from within that single file is the matching of the data strings to the multi-bit wave characters - It looks like the matching needs the multi-bit character to be at least 2 bricks wide - so, we don't get the data in multi-bit characters that are part of sub-cycles and rendered for only one brick.
Unfortunately, my build environment is a bit unconventional - My JavaScript experience is restricted to development of simple scripts called in a single HTML file and I use a lot of console log statements to debug. Apparently, the current WaveDrom build environment doesn't allow that. I had to build using the supplied instructions, copy over the generated wavedrom.js and wavedrom.min.js to a separate folder that had the editor.html from a download of the source code zip file, and test visually for the attached WaveJSON. I also didn't check for how nodes and annotations would match up with this sub-cycle feature (those don't seem to be part of the parse-wave-lane.js that I modified). Maybe I am missing some regression test that can be done to qualify the changes.
Please feel free to use / discard the patch, as you see fit.
Best Regards Ganesh
Refactored the code attached in the ZIP above to reduce eslint complexity from 23 to 21
the single file that I modified as part of my patch as a ZIP archive.
diff lib/parse-wave-lane.js lib/backed-up-parse-wave-lane.js
74c74,85
< if (!doGenFWB) {
---
> if (doGenFWB) {
> // Rendering a seamless transition or starting of the wave lane
> // for the desired number of repetitions (translated based on the period)
> if (numSegments != 0) {
> R = R.concat(genFirstWaveBrick(Next, 0, numSegments));
> }
> // Add a single brick render of the same type as the previously rendered ones
> // to handle the odd-boundary case
> if (halfCycleRender) {
> R = R.concat(genFirstWaveBrick(Next, 0, 0));
> }
> } else {
78,87c89,96
< }
< // Rendering a seamless transition or starting of the wave lane
< // for the desired number of repetitions (translated based on the period)
< if (numSegments != 0) {
< R = R.concat(genFirstWaveBrick(Next, 0, numSegments));
< }
< // Add a single brick render of the same type as the previously rendered ones
< // to handle the odd-boundary case
< if (halfCycleRender) {
< R = R.concat(genFirstWaveBrick(Next, 0, 0));
---
> // Handle the number of repetitions in the same manner as was done
> // for the seamless transition / wave lane starting case
> if (numSegments != 0) {
> R = R.concat(genFirstWaveBrick(Next, 0, numSegments));
> }
> if (halfCycleRender) {
> R = R.concat(genFirstWaveBrick(Next, 0, 0));
> }
Aliaksei,
Can you shed more light on how this sub-cycle feature is supposed to behave with non-default hscale values?
For hscale = 2 and period = 1, should we be putting in 4 bricks per period? To be more exact, will we be supporting (as example) all of <2345> in a single period, or, should it be rendered as 2,3 in the first period and 4,5 in the next one?
I have been thinking about this more, and IMO, users get the most flexibility if sub-cycle rendering is always in terms of the minimum possible brick size for the configuration. So, with hscale = 2, I think we should have each character inside a sub-cycle represent 1/4th of the period (in the example above, all of 2,3,4, and 5 would render within the space between two vertical markers). The patch above assumes hscale = 1, but, it is trivial to fix by adding the 'extra' parameter to the genWaveBrick call inside the subCycle-enabled block
if (!doGenFWB) {
// Handling an abrupt transition from one type of brick to another
// by first rendering the single brick dependent on both the old and current characters
R = R.concat(genWaveBrick((Top + Next), extra, 0));
}
However, this breaks the rendering for WaveJSON strings with gap markers
{name: "sig4", "wave": "<=|>.x" },
{name: "sig5", "wave": "x<=|>.x" },
{name: "sig6", "wave": "x<.|>0x" },
I am trying to figure out the code responsible for aligning the gap markers...
The sub-cycle consideration in render-gaps.js was not differentiating between sub-cycle nodes and regular nodes when taking hscale into consideration. I am attaching a zip version of the two files (parse-wave-lane.js and render-gaps.js) that I have modified over the last couple of days. This version renders all test cases in the repository not related to the sub-cycle feature correctly (existing functionality is not broken AFAIK). All the test cases from various bug reports related to the sub-cycle feature are rendered in a well-defined manner for different hscale configurations and period values.
parse-wave-lane-render-gaps-patch-ganeshts-10092019.zip
@drom Please let me know if there is anything else I could help with in closing Issues #240 #243 and #244 - Documentation can be made ready quickly once we agree on how hscale will affect sub-cycle rendering (I already have some PDF slides assuming the behavior I have implemented)