[PATCH] qmi-message: add functions to operate on QMI part of QmiMessage
Aleksander Morgado
aleksander at aleksander.es
Tue Feb 5 08:43:03 UTC 2019
Hey!
Sorry for the late reply, I totally forgot about this patch... :/
I think we should stick to sending Merge Requests in the fd.o gitlab,
could you please send follow ups as MRs?
See some comments below.
On Tue, Jan 8, 2019 at 8:11 PM Eric Caruso <ejcaruso at chromium.org> wrote:
>
> QRTR-based transports will not have QMUX headers, and will instead
> encode that information in a different way. However, that data will
> still need to be passed around as part of the message by the library
> consumer, so reusing the QMUX header structure to pass it around
> should be fine and we will need to be able to create fake QMUX
> headers.
>
> Also add a unit test for the new functions.
> ---
> src/libqmi-glib/qmi-message.c | 63 +++++++++++++++++++++++++++++
> src/libqmi-glib/qmi-message.h | 37 +++++++++++++++++
> src/libqmi-glib/test/test-message.c | 58 ++++++++++++++++++++++++--
> 3 files changed, 155 insertions(+), 3 deletions(-)
>
> diff --git a/src/libqmi-glib/qmi-message.c b/src/libqmi-glib/qmi-message.c
> index 7ba1dd5..b2fe177 100644
> --- a/src/libqmi-glib/qmi-message.c
> +++ b/src/libqmi-glib/qmi-message.c
> @@ -449,6 +449,53 @@ qmi_message_new (QmiService service,
> return (QmiMessage *)self;
> }
>
> +QmiMessage *
> +qmi_message_new_from_qmi (QmiService service,
I understand the reasoning behind the naming, but it does sound a bit
weird to "build a qmi message from qmi". How about
qmi_message_new_from_data() ? the reasoning could be that a QMI
message is composed of a header (the QMUX part) and data (the QMI
part). What do you think?
>
> + guint8 client_id,
> + GByteArray *raw,
> + GError **error)
> +{
> + GByteArray *self;
> + struct full_message *buffer;
> + gsize buffer_len;
> + gsize message_len;
> +
> + /* Create array with enough size for the QMUX marker and QMUX header, and
> + * with enough room to copy the rest of the message into */
> + if (service == QMI_SERVICE_CTL) {
> + message_len = sizeof (struct control_header) +
> + ((struct control_message *)(raw->data))->header.tlv_length;
> + } else {
> + message_len = sizeof (struct service_header) +
> + ((struct service_message *)(raw->data))->header.tlv_length;
> + }
> + buffer_len = (1 + sizeof (struct qmux) + message_len);
> + /* Create the GByteArray with buffer_len bytes preallocated */
> + self = g_byte_array_sized_new (buffer_len);
> + g_byte_array_set_size (self, buffer_len);
> +
> + /* Set up fake QMUX header */
> + buffer = (struct full_message *)(self->data);
> + buffer->marker = QMI_MESSAGE_QMUX_MARKER;
> + buffer->qmux.length = buffer_len - 1;
> + buffer->qmux.flags = 0;
> + buffer->qmux.service = service;
> + buffer->qmux.client = client_id;
> +
> + /* Move bytes from the raw array to the newly created message */
> + memcpy (&buffer->qmi, raw->data, message_len);
> + g_byte_array_remove_range (raw, 0, message_len);
> +
> + /* Check input message validity as soon as we create the QmiMessage */
> + if (!message_check (self, error)) {
> + /* Yes, we lose the whole message here */
> + qmi_message_unref (self);
> + return NULL;
> + }
> +
> + return (QmiMessage *)self;
> +}
> +
> QmiMessage *
> qmi_message_response_new (QmiMessage *request,
> QmiProtocolError error)
> @@ -510,6 +557,22 @@ qmi_message_get_raw (QmiMessage *self,
> return self->data;
> }
>
> +const guint8 *
> +qmi_message_get_qmi (QmiMessage *self,
And here we could have qmi_message_get_data(). I think it reads
better, don't know.
> + gsize *length,
> + GError **error)
> +{
> + g_return_val_if_fail (self != NULL, NULL);
> + g_return_val_if_fail (length != NULL, NULL);
> +
> + if (message_is_control (self))
> + *length = sizeof (struct control_header);
> + else
> + *length = sizeof (struct service_header);
> + *length += get_all_tlvs_length (self);
> + return (guint8 *)(&((struct full_message *)(self->data))->qmi);
> +}
> +
> /*****************************************************************************/
> /* TLV builder & writer */
>
> diff --git a/src/libqmi-glib/qmi-message.h b/src/libqmi-glib/qmi-message.h
> index 714adae..ddc637c 100644
> --- a/src/libqmi-glib/qmi-message.h
> +++ b/src/libqmi-glib/qmi-message.h
> @@ -122,6 +122,26 @@ QmiMessage *qmi_message_new (QmiService service,
> QmiMessage *qmi_message_new_from_raw (GByteArray *raw,
> GError **error);
>
> +/**
> + * qmi_message_new_from_qmi:
> + * @service: a #QmiService
> + * @client_id: client ID of the originating control point.
> + * @raw: (inout): raw data buffer containing only the QMI part of the message.
> + * @error: return location for error or %NULL.
> + *
> + * Create a new #QmiMessage from the given raw data buffer.
> + *
> + * Whenever a complete QMI message is read, its raw data gets removed from the @raw buffer.
> + *
> + * Returns: (transfer full): a newly created #QmiMessage, which should be freed with qmi_message_unref(). If @raw doesn't contain a complete QMI message #NULL is returned. If there is a complete QMI message but it appears not to be valid, #NULL is returned and @error is set.
> + *
> + * Since: 1.24
> + */
> +QmiMessage *qmi_message_new_from_qmi (QmiService service,
> + guint8 client_id,
> + GByteArray *raw,
> + GError **error);
> +
> /**
> * qmi_message_response_new:
> * @request: a request #QmiMessage.
> @@ -274,6 +294,23 @@ const guint8 *qmi_message_get_raw (QmiMessage *self,
> gsize *length,
> GError **error);
>
> +
> +/**
> + * qmi_message_get_raw:
> + * @self: a #QmiMessage.
> + * @length: (out): return location for the size of the output buffer.
> + * @error: return location for error or %NULL.
> + *
> + * Gets the raw data buffer of the #QmiMessage.
> + *
> + * Returns: (transfer none): The raw QMI buffer, or #NULL if @error is set.
> + *
> + * Since: 1.24
> + */
> +const guint8 *qmi_message_get_qmi (QmiMessage *self,
> + gsize *length,
> + GError **error);
> +
> /*****************************************************************************/
> /* Version support from the database */
> /**
> diff --git a/src/libqmi-glib/test/test-message.c b/src/libqmi-glib/test/test-message.c
> index 7a4b11d..a53a1d0 100644
> --- a/src/libqmi-glib/test/test-message.c
> +++ b/src/libqmi-glib/test/test-message.c
> @@ -272,6 +272,57 @@ test_message_new_request (void)
> qmi_message_unref (self);
> }
>
> +static void
> +test_message_new_request_from_qmi (void)
> +{
> + static const guint8 expected_buffer [] = {
> + 0x00, /* service flags */
> + 0x02, 0x00, /* transaction */
> + 0xFF, 0xFF, /* message id */
> + 0x00, 0x00, /* all tlvs length */
> + };
> +
> + GByteArray *qmi;
> + QmiMessage *self;
> + GError *error = NULL;
> + const guint8 *buffer;
> + gsize buffer_length = 0;
> +
> + /* set up service header */
> + qmi = g_byte_array_new ();
> + g_byte_array_append (qmi, expected_buffer, sizeof (expected_buffer));
> +
> + self = qmi_message_new_from_qmi (QMI_SERVICE_DMS, 0x01, qmi, NULL);
> + g_assert (self);
> +
> + /* check that the QMUX header contains the right values*/
> + g_assert_cmpuint (qmi_message_get_service (self), ==, QMI_SERVICE_DMS);
> + g_assert_cmpuint (qmi_message_get_client_id (self), ==, 0x01);
> +
> + /* length (13) = qmux marker (1) + qmux header length (5) + qmi header length (7) */
> + g_assert_cmpuint (qmi_message_get_length (self), ==, 13);
> +
> + /* check that transaction and message IDs line up */
> + g_assert_cmpuint (qmi_message_get_transaction_id (self), ==, 0x02);
> + g_assert_cmpuint (qmi_message_get_message_id (self), ==, 0xFFFF);
> +
> + buffer = qmi_message_get_raw (self, &buffer_length, &error);
> + g_assert_no_error (error);
> + g_assert (buffer != NULL);
> + g_assert_cmpuint (buffer_length, >, 0);
> + /* check that we have a qmux marker so we don't break framing */
> + g_assert_cmpuint (buffer[0], ==, 0x01);
> +
> + /* make sure the qmi portion of the message is what we expect */
> + buffer = qmi_message_get_qmi (self, &buffer_length, &error);
> + g_assert_no_error (error);
> + g_assert (buffer != NULL);
> + g_assert_cmpuint (buffer_length, >, 0);
> + _g_assert_cmpmem (buffer, buffer_length, expected_buffer, sizeof (expected_buffer));
> +
> + qmi_message_unref (self);
> +}
> +
> static void
> test_message_new_response_ok (void)
> {
> @@ -1514,9 +1565,10 @@ int main (int argc, char **argv)
> g_test_add_func ("/libqmi-glib/message/parse/wrong-tlv", test_message_parse_wrong_tlv);
> g_test_add_func ("/libqmi-glib/message/parse/missing-size", test_message_parse_missing_size);
>
> - g_test_add_func ("/libqmi-glib/message/new/request", test_message_new_request);
> - g_test_add_func ("/libqmi-glib/message/new/response/ok", test_message_new_response_ok);
> - g_test_add_func ("/libqmi-glib/message/new/response/error", test_message_new_response_error);
> + g_test_add_func ("/libqmi-glib/message/new/request", test_message_new_request);
> + g_test_add_func ("/libqmi-glib/message/new/request-from-qmi", test_message_new_request_from_qmi);
> + g_test_add_func ("/libqmi-glib/message/new/response/ok", test_message_new_response_ok);
> + g_test_add_func ("/libqmi-glib/message/new/response/error", test_message_new_response_error);
>
> g_test_add_func ("/libqmi-glib/message/tlv-write/empty", test_message_tlv_write_empty);
> g_test_add_func ("/libqmi-glib/message/tlv-write/reset", test_message_tlv_write_reset);
> --
> 2.20.1.97.g81188d93c3-goog
>
--
Aleksander
https://aleksander.es
More information about the libqmi-devel
mailing list