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.

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

Unexpected error when arming #57

Open
coretl opened this issue Jul 8, 2024 · 1 comment
Open

Unexpected error when arming #57

coretl opened this issue Jul 8, 2024 · 1 comment

Comments

@coretl
Copy link
Contributor

coretl commented Jul 8, 2024

Somehow we hit this TEST_OK while doing *PCAP.ARM:

/* If data capture is not enabled then we can safely expect the
* buffer status to be idle. */
TEST_OK(!read_buffer_status(data_buffer, &readers, &active)) ?:

Causing ERR: Unexpected error at .../data_server.c:220. This was the ID panda that @glennchid upgraded, so I guess this was the 3.0 release, rather than @EmilioPeJu's latest changes, although I can't see anything in them that touches this code.

There are 2 parts of the code that connect to a PandA:

  • One makes a connection to PandA and sets some parameters
  • The second makes a connection to PandA, sets some parameters, opens the data port, and as soon as it says "OK" sends "*PCAP.ARM"

Inserting a 1 second sleep between the two fixes the problem. Inserting a 0.1s sleep does not.

I have yet to reproduce this outside the application they were using, but before I do, do you know what might trigger this?

@Araneidae
Copy link
Contributor

There seems to be a state where the flag data_capture_enabled is false but the data_buffer is still active. This buffer has three states: STATE_IDLE, STATE_ACTIVE, and STATE_CLEARING, the last being used when a data capture from hardware is complete but data is still be transferred to a connected data user. The function read_buffer_status returns false when the buffer is in STATE_IDLE and true otherwise.

Hmm. I think this is probably a bug in the return value from read_buffer_status, as the next line after the failing test is testing the number of still connected clients. This function is only used twice, the second call is just a few lines further down used to implement *PCAP.STATUS?:

error__t get_capture_status(struct connection_result *result)
{
unsigned int readers, active;
bool status = read_buffer_status(data_buffer, &readers, &active);
return format_one_result(
result, "%s %u %u", status ? "Busy" : "Idle", readers, active);
}

I think the simplest fix is just

diff --git a/server/data_server.c b/server/data_server.c
index 37bba82..54cb2bc 100644
--- a/server/data_server.c
+++ b/server/data_server.c
@@ -251,7 +251,7 @@ error__t arm_capture(void)
                 "Data capture already in progress")  ?:
             /* If data capture is not enabled then we can safely expect the
              * buffer status to be idle. */
-            TEST_OK(!read_buffer_status(data_buffer, &readers, &active))  ?:
+            DO(read_buffer_status(data_buffer, &readers, &active))  ?:
             TEST_OK_(active == 0, "Data clients still taking data")  ?:
             /* Get PCAP ARM timestamp and store as a global */
             TEST_IO(clock_gettime(CLOCK_REALTIME, &pcap_arm_ts))  ?:

A more thorough fix might involve exposing all three states from this function, decoding them properly in get_capture_status, and expecting not STATE_ACTIVE in the read buffer call above, but I don't think this is worth doing.

In brief, I think your test should be failing with the message ERR: Data clients still taking data instead.

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

No branches or pull requests

2 participants