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

Kaa 773 log delivery callbacks #441

Merged
merged 18 commits into from
Jan 25, 2016
Merged

Kaa 773 log delivery callbacks #441

merged 18 commits into from
Jan 25, 2016

Conversation

forGGe
Copy link
Contributor

@forGGe forGGe commented Jan 19, 2016

Includes:

  • implementation
  • tests
  • small improvements to the test framework
  • couple of nitpicks

typedef struct
{
uint16_t log_bucket_id; /**< ID of bucket present in storage. */
kaa_time_t timeout; /**< bucket timeout */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move timeout to start to avoid padding

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done



#define KAA_MAX_LOGS_IN_BUCKET 64
#define KAA_MAX_BUCKET_SIZE 1024
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, check whether KAA_MAX_BUCKET_SIZE doesn't exceed the size of a OUT transport buffer.
See defaults.h

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


/* Logging constraints */
#define MAX_LOG_COUNT SIZE_MAX
#define MAX_LOG_BUCKET_SIZE (2 * 1024)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, check whether KAA_MAX_BUCKET_SIZE doesn't exceed the size of a OUT transport buffer.
See defaults.h

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


if (((timeout_info_t *)data)->log_bucket_id == find_context->bucket_id) {
find_context->bucket_count++;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to return log_count from timeout_info_t instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@ghost
Copy link

ghost commented Jan 21, 2016

LGTM


if (timeout_info->log_bucket_id == timeout_to_find->log_bucket_id) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid context state changes, please use kaa_list_find_next() to get the necessary info and kaa_list_remove_at() to remove the element

@GamovCoder
Copy link
Contributor

LGTM

forGGe added a commit that referenced this pull request Jan 25, 2016
@forGGe forGGe merged commit c872d1e into kaaproject:develop/v0.8.0 Jan 25, 2016
@forGGe forGGe deleted the KAA-773_log_delivery_callbacks branch January 26, 2016 10:49
@forGGe forGGe restored the KAA-773_log_delivery_callbacks branch March 30, 2016 14:50
@forGGe forGGe deleted the KAA-773_log_delivery_callbacks branch March 31, 2016 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants