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

Prepare release v3.5.0 #100

Merged
merged 18 commits into from
Feb 9, 2024
Merged

Prepare release v3.5.0 #100

merged 18 commits into from
Feb 9, 2024

Conversation

ACascarino
Copy link
Contributor

@ACascarino ACascarino commented Feb 9, 2024

Entirety of release is just that which was merged in #99

I2S tests fail. This is known as a major issue with the test runner. Can confirm that the tests pass when manually run.

@ACascarino ACascarino requested a review from mbanth February 9, 2024 11:36
@mbanth mbanth assigned ACascarino and unassigned mbanth Feb 9, 2024
Copy link
Collaborator

@mbanth mbanth left a comment

Choose a reason for hiding this comment

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

I have requested one documentation change and made one suggested (and optional) improvement.

@@ -6,6 +6,12 @@
|I2S| Instances
===============

The macro I2S_DATA_WIDTH may be set as a compile flag (e.g.
-DI2S_DATA_WIDTH=16) to alter the number of bits per word for both the I2S
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please change I2S to |I2S| at the end of this line and in lines 11 and 12.

)
clock_half_period = float(1000000000000) / float(2 * bclk_frequency)
clock_half_period = float(ONE_SECOND_FS) / float(2 * bclk_frequency)
data_bit_mask = int("1" * data_bits, base=2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Moving the definition of data_bit_mask closer to its first use might improve maintainability slightly. The first use is over 100 lines away.

@ACascarino ACascarino merged commit 10bb117 into release/v3.5.0 Feb 9, 2024
3 checks passed
@mbanth mbanth mentioned this pull request Feb 9, 2024
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