Skip to content

Conversation

@bparks13
Copy link
Member

This PR modifies the console output to correctly identify the PCIe Host firmware version. Previously it was reported as the breakout board, which was incorrect.

Fixes #150

@bparks13 bparks13 added this to the 0.2.1 milestone Oct 24, 2025
@bparks13 bparks13 self-assigned this Oct 24, 2025
@bparks13 bparks13 modified the milestones: 0.2.1, 0.3.0 Oct 27, 2025
@bparks13 bparks13 requested a review from cjsha October 30, 2025 19:49
Copy link
Collaborator

@cjsha cjsha left a comment

Choose a reason for hiding this comment

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

I think in the future, we want to refer to the ONIX PCIe card as the ONIX Controller and the host is the computer where the controller is plugged in. This is my understanding from this issue in the onix-docs anyway. I suspect we should go with the nomenclature indicated in that issue.

In line with one of the comments you deleted in the PR, you can probably delete the comment on this line as well:

if (hubId == ONIX_HUB_FMCHOST) // NB: Breakout Board

I guess it's not necessary to check the breakout's firmware for compatibility? only the controller's firmware?

I'll approve this PR, but please consider these comments and make the appropriate changes before merging. Feel free to re-request a review from me if you want me to take another look.

@bparks13
Copy link
Member Author

bparks13 commented Nov 3, 2025

@cjsha you bring up a good point about the PCIe card vs. Controller issue, I was using the ONIX docs as a reference, but we want to change that nomenclature in the future. We will have to keep an eye on it so we can change the terminology here when it is updated in the docs.

Good catch with the other comment! I totally missed that

We should add checks for all hardware, in fact it is quite similar to what we recently merged with the Onix1 library where we enforce a minimum version in all devices, since there is an implicit version we are developing against we should instead make it explicit.

@bparks13 bparks13 merged commit da66dd7 into main Nov 3, 2025
3 checks passed
@bparks13 bparks13 deleted the issue-150 branch November 3, 2025 18:28
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.

Console reports breakout board fw but it should be host fw

3 participants