Skip to content

Add hard and soft reset support to riffa onidriver #40

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

aacuevas
Copy link
Collaborator

Required for:
open-ephys/onix-common-hdl#26
open-ephys/onix-pcie-host-hdl#13

@aacuevas aacuevas requested a review from jonnew May 27, 2025 00:18
@jonnew
Copy link
Member

jonnew commented May 30, 2025

I've now tested this on

  • Headstage-64 alone
  • NPX 1.0f alone
  • Headstage-64 and NPX 1.0f together
  • Headstage-64 and NPX 1.0e (passthrough) together

all these seem to work with incident.

@aacuevas
Copy link
Collaborator Author

Regarding versioning:
After much thought, I believe libonidriver_riffa should not be 2.0.0 but 1.1.0
The reason being, this onidriver seems to work well with older firmware, so it does not really break compatibility. And the software API does not change. However, it introduces a new "feature" (i.e.: hardware behavior) that is required by the new firmware (the firmware does break compatibility with older drivers, so it is a major update)

For the same reasons, clroni should increase to 6.2.0

@jonnew what do you think? Versioning is a bit of a headache in this case, but I believe the rule is "if it breaks compatibility in any direction (up or downstream) then it's a major, if it retains compatibility in both directions but adds something in any, minor, if it's just a fix, patch"

@jonnew
Copy link
Member

jonnew commented May 31, 2025

I came here to follow up with exactly this question: does this work with old firmware? Since it does, the major version should not increment.

So, I agree with the proposed changes. The only thing I want to make sure i understand is the "direction, up stream, downstream", wording. I don't think we need to think about this at all assuming it refers to a "spatial" dimension in some sense. We just need to think about two things:

  1. The public interface which is very nicely defined for all of our components
  2. If that public interface works with the exterior components that the last version of the interface did work with.

If the answer to 2. is no, then major increase.

So, in this case

  1. HDL: major increment
  2. Driver translator: minor increment
  3. liboni: No increment
  4. clroni: minor increment

@aacuevas
Copy link
Collaborator Author

aacuevas commented May 31, 2025

Well, what I meant here is, there are two public interfaces in the driver translator or in libONI: The software API and the hardware communication. So, in a sense, there are two interfaces, if any of these broke (e.g. this translator did not work with older gateware) then it would be a major update regardless of the other.

In this case, we should do some stress-test to be sure that, indeed, the driver translator does work with current gateware. If it indeed does, it would be exactly the case you wrote (so the driver translator should be 1.2.0 instead of the 2.0.0 I wrote. should not be an issue since this is not published yet)

@jonnew
Copy link
Member

jonnew commented May 31, 2025

Ah, yes. TBH, i was starting to think along those lines even when writing this. However, I think both points are correct: the hardware and and software interfaces both form the greater public interface. If any part of that public interface is does not work with whatever was was previously "plugged in" to it, the its a major change.

But I do understand the wording now.

I think it might be worth making a set of tests on Monday's meeting to ensure that we are testing all the recent changes properly.

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