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

Variable-sized registers not doing the expected thing #883

Open
dkogan opened this issue Mar 13, 2024 · 7 comments
Open

Variable-sized registers not doing the expected thing #883

dkogan opened this issue Mar 13, 2024 · 7 comments
Labels
1. bug Problems, incorrect behavior or appearance 4. Help wanted Help is welcome to resolve the issue 5. Genicam Issue in Genicam implementation

Comments

@dkogan
Copy link

dkogan commented Mar 13, 2024

Hello. I'm looking at getting the bulk transfers working for my sierra-olympia tenum-640 cameras talking via a gige pleora frame grabber; I asked about this on the forum: https://aravis-project.discourse.group/t/how-to-execute-bulk-serial-transfers-in-aravis/728

I ran the vendor's software and collected a packet trace to see what "right" behavior looks like. It looks like aravis has issues with both sending and receiving the data. This bug report is about the sending.

I'm using aravis-0.8.30.

The data is sent via this register (snippet from arv-tool-0.8 genicam):

		<Register Name="Bulk0DownstreamData">
			<ToolTip>Allows sending data to the peripheral connected to the Bulk 0 interface.</ToolTip>
			<Description>Allows sending data to the peripheral connected to the Bulk 0 interface. Users must not write data directly into this feature.</Description>
			<DisplayName>Downstream Data</DisplayName>
			<Visibility>Invisible</Visibility>
			<pIsImplemented>Bulk0IsAvailable</pIsImplemented>
			<Address>0x40058000</Address>
			<Length>33554432</Length>
			<AccessMode>WO</AccessMode>
			<pPort>Device</pPort>
			<Cachable>WriteThrough</Cachable>

I know it says that I'm not supposed to write data directly into this feature, so maybe I'm not doing it right. However, this is what I'm doing:

    const char payload[] = {...};
    ArvGcNode* feature = arv_device_get_feature(device, "Bulk0DownstreamData");
    arv_gc_register_set(ARV_GC_REGISTER(feature),
                        payload, sizeof(payload),
                        &error);

It mostly does the right thing. I see the WRITEMEM_CMD, but instead of writing sizeof(payload) bytes, it writes 33554432 bytes, as noted in the .xml. Am I supposed to be doing this differently? Should aravis respect the given length?

Thank you very much for aravis!

@EmmanuelP
Copy link
Contributor

Hi @dkogan

Aravis is implemented by reverse engineering the protocols. It seemed sensible to always use the feature Length property to determine the transfer size. But obviously in this case that does not make sense.

We could try to adjust the transfer size to what is actually needed, but I don't know if it is a generally valid behaviour. Any help in testing this idea will be appreciated. The related code is here:

static void
arv_gc_register_node_get (ArvGcRegister *gc_register, void *buffer, guint64 length, GError **error)
{
ArvGcRegisterNode *gc_register_node = ARV_GC_REGISTER_NODE (gc_register);
GError *local_error = NULL;
void *cache;
gint64 address;
gint64 cache_length;
cache = _get_cache (gc_register_node, &address, &cache_length, &local_error);
if (local_error != NULL) {
g_propagate_error (error, local_error);
return;
}
if (length < cache_length) {
g_set_error (error, ARV_GC_ERROR, ARV_GC_ERROR_INVALID_LENGTH,
"[%s] Register get failed due to data not fitting into buffer",
arv_gc_feature_node_get_name (ARV_GC_FEATURE_NODE (gc_register)));
return;
}
_read_from_port (gc_register_node, address, cache_length, cache, _get_cachable (gc_register_node), &local_error);
if (local_error != NULL) {
g_propagate_error (error, local_error);
return;
}
if (length > cache_length) {
memcpy (buffer, cache, cache_length);
memset (((char *) buffer) + cache_length, 0, length - cache_length);
} else
memcpy (buffer, cache, length);
arv_debug_genicam ("[GcRegisterNode::get] 0x%" G_GINT64_MODIFIER "x,%" G_GUINT64_FORMAT, address, length);
}
static void
arv_gc_register_node_set (ArvGcRegister *gc_register, const void *buffer, guint64 length, GError **error)
{
ArvGcRegisterNode *gc_register_node = ARV_GC_REGISTER_NODE (gc_register);
GError *local_error = NULL;
void *cache;
gint64 address;
gint64 cache_length;
cache = _get_cache (gc_register_node, &address, &cache_length, &local_error);
if (local_error != NULL) {
g_propagate_error (error, local_error);
return;
}
if (cache_length < length) {
g_set_error (error, ARV_GC_ERROR, ARV_GC_ERROR_INVALID_LENGTH,
"[%s] Register set failed due to data not fitting into register",
arv_gc_feature_node_get_name (ARV_GC_FEATURE_NODE (gc_register)));
return;
}
if (cache_length > length) {
memcpy (cache, buffer, length);
memset (((char *) cache) + length, 0, cache_length - length);
} else
memcpy (cache, buffer, cache_length);
_write_to_port (gc_register_node, address, cache_length, cache, _get_cachable (gc_register_node), &local_error);
if (local_error != NULL) {
g_propagate_error (error, local_error);
return;
}
arv_debug_genicam ("[GcRegisterNode::set] 0x%" G_GINT64_MODIFIER "x,%" G_GUINT64_FORMAT, address, length);
}

Meanwhile, you can probably use arv_device_write_memory() and arv_gc_register_get_address() as a workaround.

@EmmanuelP EmmanuelP added 1. bug Problems, incorrect behavior or appearance 5. Genicam Issue in Genicam implementation 4. Help wanted Help is welcome to resolve the issue labels Mar 13, 2024
@dkogan
Copy link
Author

dkogan commented Mar 14, 2024

Thanks for the suggestion. I tried arv_device_write_memory() and arv_gc_register_get_address(), and they are an effective workaround.

When I look at the "good" packet trace, I see the payload data being preceded by a 64-bit-big-endian integer indicating the payload size. Since aravis pads the data to 32-bit chunks, this extra field feels required: otherwise the true payload size isn't present anywhere.

In the "good" packet trace, the camera responds to the data sent in this register, but I cannot yet reproduce this with aravis, even with patches to make the outgoing packets match. I'm not going to propose any patches until I can make this work.

Thanks much

@freeman94
Copy link
Contributor

Hey @dkogan, I happen to be in the exact same situation as you. Please let me know what progress you are able to make. I will be working on this problem as well.

@freeman94
Copy link
Contributor

To follow up on this, I have been able to successfully control the tenum 640 via Bulk0DownstreamData using the arv_device_write_memory() workaround suggestion. My packet trace does not contain the extra payload size field but the camera accepts the command regardless. I can provide additional details if desired.

@dkogan
Copy link
Author

dkogan commented Mar 26, 2024

Oh wow. I definitely would like more info. Like I said, I tweaked the code to add the "payload-size" field, and the packet traces then showed that I'm sending the right thing. But the camera still wasn't responding (no reply in the packet trace). I was doing other things, and haven't tried to debug it yet. And it looked like extra work in aravis would be required to be able to read and interpret the responses (#491).

Let me send you the packet traces I see, so that we can compare. Questions:

  • If you aren't seeing a payload-size field, then our cameras are somehow talking a slightly different protocol. Can I get a packet trace from you that shows some of your communication?
  • Did you have to do anything extra to make the camera respond?
  • Are you able to read the responses?

Thanks!

@freeman94
Copy link
Contributor

Let me clarify that I have also not gotten the camera's response to the serial command. Event support would be needed for this to work of course, but I have also not fully investigated all the settings necessary to get the pleora device to at least send the event packet. Anyway here's a packet trace to show everything I've done to send a serial command (sets 8bit mono output mode) and have the camera successfully execute the command. I can also share a code snippet that produced this.

8bitMonoShift_capture.zip

@dkogan
Copy link
Author

dkogan commented Mar 27, 2024

Ah. OK. Here's a trace from using the eBUSPlayer, where the data is sent and received properly. The send is event 369 and the reply is event 371. Note the 64-bit size field in event 369. Like I said, I patched my code to produce the same send data (including the size and the 0 padding), but I don't get a reply. Will debug it later.

uart-good-from-ebusplayer.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. bug Problems, incorrect behavior or appearance 4. Help wanted Help is welcome to resolve the issue 5. Genicam Issue in Genicam implementation
Projects
None yet
Development

No branches or pull requests

3 participants