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

Extend tests to exercise the BCLK adjustment argument #20

Open
xhuw opened this issue Feb 21, 2023 · 5 comments
Open

Extend tests to exercise the BCLK adjustment argument #20

xhuw opened this issue Feb 21, 2023 · 5 comments

Comments

@xhuw
Copy link
Contributor

xhuw commented Feb 21, 2023

Current tests don't use BCLK adjustment. Would be nice to test this feature.

@mbanth
Copy link
Contributor

mbanth commented Feb 21, 2023

@xhuw, please provide a Story Point value for this issue.

@ed-xmos
Copy link
Contributor

ed-xmos commented Feb 21, 2023

It seems this is being done already so I think we can close this.

Edit - we do exercise the logic path that checks this but we don't specifically test scaling of anything other that 1.0.

@mbanth
Copy link
Contributor

mbanth commented Feb 21, 2023

It seems this is being done already so I think we can close this.

Edit - we do exercise the logic path that checks this but we don't specifically test scaling of anything other that 1.0.

@xhuw, do you agree with @ed-xmos or think that further tests are needed?

@xhuw
Copy link
Contributor Author

xhuw commented Feb 22, 2023

@mbanth it would be good to test this feature explicitly. I didn't originally because it seemed like more effort than it was worth, but it would have caught an issue we found on Ed's branch quicker yesterday if I had put the test in.
quite low priority I think though as the tests do already get good coverage.

@mbanth
Copy link
Contributor

mbanth commented Feb 22, 2023

@mbanth it would be good to test this feature explicitly. I didn't originally because it seemed like more effort than it was worth, but it would have caught an issue we found on Ed's branch quicker yesterday if I had put the test in. quite low priority I think though as the tests do already get good coverage.

Thanks @xhuw for your opinion. Based on it, I will leave this issue open and in your name as part of the HRK7 deliverable and with 'Should' priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants