neurogears / onix-refactor

A project for refactoring Bonsai.ONIX
MIT License
2 stars 3 forks source link

Multiple hardware resets cause instability #119

Open jonnew opened 1 week ago

jonnew commented 1 week ago

This function is called anywhere from 2 to 5 times during headstage initialization:

https://github.com/neurogears/onix-refactor/blob/317cbc72e2232ede0b6e5ea550db05c20b60ccc5/OpenEphys.Onix/OpenEphys.Onix/ContextTask.cs#L63

Its hard to follow where its being triggered from because it seems to be registered to events that are fired during startup. In any case, this causes instability because headstages are getting hit with lots of reset signals in quick succession. When we use the C library, and the the context lifespan is 1-1 with the program itself, these instabilities are not present.

There are some headstages that require a reset after they have established a lock (rhs and nric1384 headstages for example). In oni-repl, this means the program has to be closed and reopened after the port voltage established. Not convenient but works every time.

We need a similar amount of determinism with this library. Initialize should be performed exactly two times. Once at the start of acquisition to get the local device table. A second time after all port voltages have been applied.

aacuevas commented 5 days ago

As with most things, passthrough devices add to our pain. They require an extra, annoying, init. AFAIK, the general case uses 4 resets, for these specific reasons:

  1. Initial reset. Passthrough bit, if required, needs to be set here
  2. Local device table retrieval, with any passthrough device if enabled. We then perform port voltage probing
  3. Retrieval of full device table, including remote devices from non-passthrough headstages. Registers that require a reset (e.g. Enable, the loadtester's payload sizes, etc... but mainly and always Enable bits) are set here
  4. Final reset to make all the enables and other reset-requiring registers active

From when we designed this chain of events, the resets should be deterministic to this amount, not sure what might have changed or if some might be skipped now, but the general case requires these 4 no matter what.

glopesdev commented 3 days ago

@jonnew Maybe we should have a list of problematic devices? I noticed in some cases such as the RHS2116 that the link controller implementation itself is calling an extra context reset which may explain why you were getting 5 resets instead of 4:

https://github.com/neurogears/onix-refactor/blob/ea49ff579386107330bd52188d47a8d88c99d09b/OpenEphys.Onix/OpenEphys.Onix/ConfigureHeadstageRhs2116.cs#L102-L103

This is maybe a dangerous way to do it since adding for example two RHS2116 headstages would result in two extra context reset calls. Maybe we need an extra deterministic step that is configured only for some devices?

glopesdev commented 3 days ago

Related to the above, another option I was wondering to get the RHS2116 working is perhaps we could move the last CheckLinkState call to run after the final context Reset? That way we might be able to avoid problematic initialization behavior such as the RHS2116 without having to introduce additional resets.