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.

Sign up for GitHub

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

Jump to bottom

Add test for coerced type (read/write API for FPGA) #1134

Merged
merged 2 commits into from
Dec 18, 2024
Merged

Add test for coerced type (read/write API for FPGA) #1134

merged 2 commits into from
Dec 18, 2024

Conversation

amehra-ni
Copy link
Contributor

@amehra-ni amehra-ni commented Nov 29, 2024 •
edited by doshirohan
Loading

What does this Pull Request accomplish?

The int32 and int64 implementations are similar as neither of them are coerced. Therefore, I have updated the int32 test to an int16 test to ensure we can test both behaviors.

Why should this Pull Request be merged?

  • For the read API, I updated the read type to int16_t to match what is read from the real API.
  • For the write API, I updated type for data written to pb::int16, even though proto expects i32, as that's how user applications will have data they want to write.

What testing has been done?

  • All tests pass successfully.

@amehra-ni amehra-ni requested a review from doshirohan November 29, 2024 08:39
@amehra-ni amehra-ni requested a review from asumit December 6, 2024 09:41
@christag-ni
Copy link
Contributor

We are planning to create a release branch on the 18th, and your PR will need to be completed before then for the changes to be in effect. Do you want these changes included in this release?

@doshirohan
fix sideband test to use i16 instead of i32 APIs, updated data types …
b0a455d 
…for Write* APIs to be I16 because that's how users code will look like
@doshirohan doshirohan marked this pull request as ready for review December 18, 2024 06:58
@christag-ni
Copy link
Contributor

I'll be creating the release branch at 11:30 CST.

@doshirohan doshirohan merged commit b1a0df4 into main Dec 18, 2024
9 of 10 checks passed
@doshirohan doshirohan deleted the users/amehra/convert-to-coerced branch December 18, 2024 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reckenro reckenro reckenro approved these changes

@doshirohan doshirohan doshirohan approved these changes

@asumit asumit Awaiting requested review from asumit

@astarche astarche Awaiting requested review from astarche astarche is a code owner

@maxxboehme maxxboehme Awaiting requested review from maxxboehme maxxboehme is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@amehra-ni @christag-ni @doshirohan @reckenro