Skip to content

Conversation

@electropaultje
Copy link

This pull request should fix issue 8.
It fixes a problem with the test bench in which a sine wave should be generated.

The rest of the fix is in the sampling of the m_data signal:

  1. Time between assertion of the amplitude_valid signal due to counter[0] and counter[1] was not constant. That is fixed by having both counter[0] and counter[1] behave exactly the same, and giving counter[1] an initial offset.
  2. m_data is added to sample_counter in 128 ticks of m_clk (m_clk_en asserted, posedge clk). In case m_data is 1 during all 128 ticks, the 7-bit sample_counter will overflow and this results in a dip in the amplitude signal (see images in the issue). This is fixed by sampling only during 127 ticks.

@electropaultje electropaultje changed the title Fix issue 8 ch5 audio Fix issue #8 ch5 audio Dec 26, 2023
@electropaultje electropaultje changed the title Fix issue #8 ch5 audio Fixes #8 ch5 audio Dec 26, 2023
@asicguy asicguy self-assigned this Dec 26, 2023
@asicguy
Copy link
Collaborator

asicguy commented Dec 26, 2023

Thanks for the contribution. I'll take a look. I would also point you to the 2nd edition of the books code, which is more accurate to how Digilent did their's. https://github.com/PacktPublishing/The-FPGA-Programming-Handbook-Second-Edition.

@electropaultje
Copy link
Author

Ah I was not aware of that. Thank you very much!

@electropaultje
Copy link
Author

Thanks for the contribution. I'll take a look. I would also point you to the 2nd edition of the books code, which is more accurate to how Digilent did their's. https://github.com/PacktPublishing/The-FPGA-Programming-Handbook-Second-Edition.

I am reading the first edition, on O'Reilly. Is the source code for the 2nd edition reasonably compatible with the 1st edition?

@asicguy
Copy link
Collaborator

asicguy commented Dec 27, 2023 via email

@electropaultje
Copy link
Author

The second edition isn't completed yet, still going through the review process. It mostly follows the first version with the addition of VHDL examples. The audio portion is, unfortunately, one of the more major deviations since we realized that the implementation deviated quite a bit from the way Digilent did it and we wanted to bring it in line. I will take a look at what you have submitted as it probably fixes real problems and get's the design into a better state. I would suggest reaching out to Packt and asking if you could be a reviewer of the second edition if you are so interested as I believe the review process is still on going. I'll bring it up to them also if you are interested.

________________________________ From: electropaultje @.> Sent: Wednesday, December 27, 2023 4:26:20 AM To: PacktPublishing/Learn-FPGA-Programming Cc: Frank Bruno; Assign Subject: Re: [PacktPublishing/Learn-FPGA-Programming] Fixes #8 ch5 audio (PR #9) Thanks for the contribution. I'll take a look. I would also point you to the 2nd edition of the books code, which is more accurate to how Digilent did their's. https://github.com/PacktPublishing/The-FPGA-Programming-Handbook-Second-Edition. I am reading the first edition, on O'Reilly. Is the source code for the 2nd edition reasonably compatible with the 1st edition? — Reply to this email directly, view it on GitHub<#9 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABZNYU4WOQPFTPGKMC6BMM3YLPZUZAVCNFSM6AAAAABBDBTDDOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZQGE3DKNJQHA. You are receiving this because you were assigned.Message ID: @.>

I am definitely interested, thanks.
But I must clarify that I'm still going over the first edition, but can definitely try to see if I can add any value by reviewing.
I'll contact Packt via the "Contact Us" form on their site. If you would like to bring it up to them as well, please do.

Thank you and have a good holiday season!

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants