Skip to content

Added event support#4

Merged
stefanrammo merged 4 commits intomainfrom
AddedEventSupport
Mar 24, 2025
Merged

Added event support#4
stefanrammo merged 4 commits intomainfrom
AddedEventSupport

Conversation

@stefanrammo
Copy link
Copy Markdown
Collaborator

Added support for querying events. Used de-duplication to prevent the same event from logging multiple times in the event.js file. Wrote a test for Time Sync to see whether it can be changed from True to False after Client object construction and after a connection has been established.

Comment thread client.js
setEnableTimeSync(enable) {
this.enableTimeSync = enable;
if (!enable) {
// Cancel any pending time sync requests so they won’t update timeDiff later.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If !enable cancels, shouldn't the opposite (enable==true) start new time sync requests?

Comment thread client.js
* codeMask: 0,
* limit: 100,
* offset: 0,
* flags: 1
Copy link
Copy Markdown
Contributor

@Karmo7 Karmo7 Mar 24, 2025

Choose a reason for hiding this comment

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

Flags should have some constants defined. For a new user the flags: 1 probably does not say much.

In C++ code, we have an enum like this:

enum class EventQueryFlags {
  None = 0,
  NewestFirst = 1,
  TimeRangeBeginExclusive = 2,
  TimeRangeEndExclusive = 4,
  UseLogstampForTimeRange = 8
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread event.js
// -----------------------------------------------------------------------
// Deduplicate events:
// Use only the sender and the event's data (after normalizing and ordering)
// to build a composite key. This ignores differences in timestamp, code,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This deduplications by timestamp and code - while it does work, I'm not sure it is that useful. I think normally a person would want to see the full event history.

Also, removing it would make the example code shorter, so it will be easier to follow.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similarly, I would also remove the normalizeValue to make the example code shorter.

Comment thread event.js
console.log(`Showing ${uniqueEvents.length} unique events:\n`);
for (const evt of uniqueEvents) {
console.log(`Timestamp: ${evt.timestampSec}`);
console.log(`Code: ${evt.code}`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think for usability, the client.js should contain some helper method to convert the event code to a human-readable string. Kind of like CDP Studio Event History pane does:

image

Common CDP event codes are listed here: https://cdpstudio.com/manual/cdp/cdplogger/eventlogreader.html#cdp-event-code-flags

But features like this can be another future pull request

Comment thread client.js
/**
* Request events based on the provided query parameters.
* @param {Object} query - An object matching the EventQuery schema.
* For example:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Example should also include how to set filters by event sender (the Source column in CDP Studio Event History Pane) and how to set filters by the data field - I mean the sender_conditions and data_conditions in the database.proto file.

message EventQuery {
  enum MatchType {
    Exact = 0;
    Wildcard = 1;
  }

  message Condition {
    optional string value = 1;
    optional MatchType type = 2;
  }

  message ConditionList {
    repeated Condition conditions = 1;
  }

  optional double time_range_begin = 1;
  optional double time_range_end = 2;
  optional uint32 code_mask = 3;
  optional uint32 limit = 4;
  optional uint32 offset = 5;
  optional uint32 flags = 6;
  optional ConditionList sender_conditions = 7;
  map<string, ConditionList> data_conditions = 8;
}

Comment thread event.js
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would move the example files event.js and index.js to an examples folder - that would make it more clear that the client.js is the actual code

@stefanrammo stefanrammo merged commit 6ef60ca into main Mar 24, 2025
@stefanrammo stefanrammo deleted the AddedEventSupport branch March 24, 2025 12:59
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

Successfully merging this pull request may close these issues.

2 participants