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.

Sign up for GitHub

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

variables improvements #199

Merged
merged 10 commits into from
Jan 8, 2025
Merged

variables improvements #199

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d085740
Updated VariablesRequest and tests with new values
chrisdp Nov 14, 2024
6add727
Inital pass at updated VariablesResponse to support vertial variables
chrisdp Nov 14, 2024
e48948b
attempting to ask for and use virtual variables
chrisdp Nov 14, 2024
a3bd0b4
Slightly better handling of virtual variables (aa keys are good, can'…
TwitchBronBron Nov 14, 2024
08d92e5
Fixed virtual vars not being inspectable and some more tests
chrisdp Nov 15, 2024
33a902e
Updated virtual variable naming convention in telnet adapter
chrisdp Nov 18, 2024
7179126
Update src/debugProtocol/events/requests/VariablesRequest.ts
chrisdp Nov 18, 2024
3d47183
Fix typo
TwitchBronBron Jan 8, 2025
546bdc8
Merge branch 'master' of https://github.com/rokucommunity/roku-debug …
TwitchBronBron Jan 8, 2025
7f0c8af
Merge branch 'master' of https://github.com/rokucommunity/roku-debug …
TwitchBronBron Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 104 additions & 18 deletions src/debugProtocol/events/requests/VariablesRequest.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ describe('VariablesRequest', () => {
requestId: 3,
getChildKeys: true,
enableForceCaseInsensitivity: false,
getVirtualKeys: false,
stackFrameIndex: 1,
threadIndex: 2,
variablePathEntries: [
{ name: 'a', forceCaseInsensitive: true },
{ name: 'b', forceCaseInsensitive: true },
{ name: 'c', forceCaseInsensitive: true }
{ name: 'a', forceCaseInsensitive: true, isVirtual: false },
{ name: 'b', forceCaseInsensitive: true, isVirtual: false },
{ name: 'c', forceCaseInsensitive: true, isVirtual: false }
]
});

Expand All @@ -24,12 +25,14 @@ describe('VariablesRequest', () => {

getChildKeys: true,
enableForceCaseInsensitivity: false,
getVirtualKeys: false,
includesVirtualPath: false,
stackFrameIndex: 1,
threadIndex: 2,
variablePathEntries: [
{ name: 'a', forceCaseInsensitive: false },
{ name: 'b', forceCaseInsensitive: false },
{ name: 'c', forceCaseInsensitive: false }
{ name: 'a', forceCaseInsensitive: false, isVirtual: false },
{ name: 'b', forceCaseInsensitive: false, isVirtual: false },
{ name: 'c', forceCaseInsensitive: false, isVirtual: false }
]
});

Expand All @@ -43,13 +46,15 @@ describe('VariablesRequest', () => {
//variable_request_flags // 1 byte
getChildKeys: true, // 0 bytes
enableForceCaseInsensitivity: false, // 0 bytes
getVirtualKeys: false,
includesVirtualPath: false,
stackFrameIndex: 1, // 4 bytes
threadIndex: 2, // 4 bytes
// variable_path_len // 4 bytes
variablePathEntries: [
{ name: 'a', forceCaseInsensitive: false }, // 2 bytes
{ name: 'b', forceCaseInsensitive: false }, // 2 bytes
{ name: 'c', forceCaseInsensitive: false } // 2 bytes
{ name: 'a', forceCaseInsensitive: false, isVirtual: false }, // 2 bytes
{ name: 'b', forceCaseInsensitive: false, isVirtual: false }, // 2 bytes
{ name: 'c', forceCaseInsensitive: false, isVirtual: false } // 2 bytes
]
});
});
Expand All @@ -59,12 +64,13 @@ describe('VariablesRequest', () => {
requestId: 3,
getChildKeys: false,
enableForceCaseInsensitivity: true,
getVirtualKeys: false,
stackFrameIndex: 1,
threadIndex: 2,
variablePathEntries: [
{ name: 'a', forceCaseInsensitive: true },
{ name: 'b', forceCaseInsensitive: false },
{ name: 'c', forceCaseInsensitive: true }
{ name: 'a', forceCaseInsensitive: true, isVirtual: false },
{ name: 'b', forceCaseInsensitive: false, isVirtual: false },
{ name: 'c', forceCaseInsensitive: true, isVirtual: false }
]
});

Expand All @@ -75,12 +81,14 @@ describe('VariablesRequest', () => {

getChildKeys: false,
enableForceCaseInsensitivity: true,
getVirtualKeys: false,
includesVirtualPath: false,
stackFrameIndex: 1,
threadIndex: 2,
variablePathEntries: [
{ name: 'a', forceCaseInsensitive: true },
{ name: 'b', forceCaseInsensitive: false },
{ name: 'c', forceCaseInsensitive: true }
{ name: 'a', forceCaseInsensitive: true, isVirtual: false },
{ name: 'b', forceCaseInsensitive: false, isVirtual: false },
{ name: 'c', forceCaseInsensitive: true, isVirtual: false }
]
});

Expand All @@ -94,21 +102,94 @@ describe('VariablesRequest', () => {
//variable_request_flags // 1 byte
getChildKeys: false, // 0 bytes
enableForceCaseInsensitivity: true, // 0 bytes
getVirtualKeys: false,
includesVirtualPath: false,
stackFrameIndex: 1, // 4 bytes
threadIndex: 2, // 4 bytes
// variable_path_len // 4 bytes
variablePathEntries: [
{
name: 'a', // 2 bytes
forceCaseInsensitive: true // 1 byte
forceCaseInsensitive: true, // 1 byte
isVirtual: false // 0 byte
}, // ?
{
name: 'b', // 2 bytes
forceCaseInsensitive: false // 1 byte
forceCaseInsensitive: false, // 1 byte
isVirtual: false // 0 byte
}, // ?
{
name: 'c', // 2 bytes
forceCaseInsensitive: true // 1 byte
forceCaseInsensitive: true, // 1 byte
isVirtual: false // 0 byte
} // ?
]
});
});

it('serializes and deserializes properly for case isVirtual lookups', () => {
const command = VariablesRequest.fromJson({
requestId: 3,
getChildKeys: false,
enableForceCaseInsensitivity: true,
getVirtualKeys: true,
stackFrameIndex: 1,
threadIndex: 2,
variablePathEntries: [
{ name: 'a', forceCaseInsensitive: true, isVirtual: true },
{ name: 'b', forceCaseInsensitive: false, isVirtual: false },
{ name: 'c', forceCaseInsensitive: true, isVirtual: true }
]
});

expect(command.data).to.eql({
packetLength: undefined,
requestId: 3,
command: Command.Variables,

getChildKeys: false,
enableForceCaseInsensitivity: true,
getVirtualKeys: true,
includesVirtualPath: true,
stackFrameIndex: 1,
threadIndex: 2,
variablePathEntries: [
{ name: 'a', forceCaseInsensitive: true, isVirtual: true },
{ name: 'b', forceCaseInsensitive: false, isVirtual: false },
{ name: 'c', forceCaseInsensitive: true, isVirtual: true }
]
});

expect(
VariablesRequest.fromBuffer(command.toBuffer()).data
).to.eql({
packetLength: 37, // 4 bytes
requestId: 3, // 4 bytes
command: Command.Variables, // 4 bytes,

//variable_request_flags // 1 byte
getChildKeys: false, // 0 bytes
enableForceCaseInsensitivity: true, // 0 bytes
getVirtualKeys: true, // 0 bytes
includesVirtualPath: true, // 0 bytes
stackFrameIndex: 1, // 4 bytes
threadIndex: 2, // 4 bytes
// variable_path_len // 4 bytes
variablePathEntries: [
{
name: 'a', // 2 bytes
forceCaseInsensitive: true, // 1 byte
isVirtual: true // 1 byte
}, // ?
{
name: 'b', // 2 bytes
forceCaseInsensitive: false, // 1 byte
isVirtual: false // 1 byte
}, // ?
{
name: 'c', // 2 bytes
forceCaseInsensitive: true, // 1 byte
isVirtual: true // 1 byte
} // ?
]
});
Expand All @@ -119,6 +200,7 @@ describe('VariablesRequest', () => {
requestId: 3,
getChildKeys: false,
enableForceCaseInsensitivity: true,
getVirtualKeys: false,
stackFrameIndex: 1,
threadIndex: 2,
variablePathEntries: []
Expand All @@ -131,6 +213,8 @@ describe('VariablesRequest', () => {

getChildKeys: false,
enableForceCaseInsensitivity: true,
getVirtualKeys: false,
includesVirtualPath: false,
stackFrameIndex: 1,
threadIndex: 2,
variablePathEntries: []
Expand All @@ -146,6 +230,8 @@ describe('VariablesRequest', () => {
//variable_request_flags // 1 byte
getChildKeys: false, // 0 bytes
enableForceCaseInsensitivity: true, // 0 bytes
getVirtualKeys: false,
includesVirtualPath: false,
stackFrameIndex: 1, // 4 bytes
threadIndex: 2, // 4 bytes
// variable_path_len // 4 bytes
Expand Down
56 changes: 54 additions & 2 deletions src/debugProtocol/events/requests/VariablesRequest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,19 @@ export class VariablesRequest implements ProtocolRequest {
requestId: number;
getChildKeys: boolean;
enableForceCaseInsensitivity: boolean;
getVirtualKeys: boolean;
threadIndex: number;
stackFrameIndex: number;
variablePathEntries: Array<{
name: string;
forceCaseInsensitive: boolean;
isVirtual: boolean;
}>;
}) {
const request = new VariablesRequest();
protocolUtil.loadJson(request, data);
request.data.variablePathEntries ??= [];
request.data.includesVirtualPath = request.data.variablePathEntries.some((entry) => entry.isVirtual);
// all variables will be case sensitive if the flag is disabled
for (const entry of request.data.variablePathEntries) {
if (request.data.enableForceCaseInsensitivity !== true) {
Expand All @@ -28,6 +31,9 @@ export class VariablesRequest implements ProtocolRequest {
//default any missing values to false
entry.forceCaseInsensitive ??= false;
}

//default any missing values to false
entry.isVirtual ??= false;
}
return request;
}
Expand All @@ -41,6 +47,8 @@ export class VariablesRequest implements ProtocolRequest {

request.data.getChildKeys = !!(variableRequestFlags & VariableRequestFlag.GetChildKeys);
request.data.enableForceCaseInsensitivity = !!(variableRequestFlags & VariableRequestFlag.CaseSensitivityOptions);
request.data.getVirtualKeys = !!(variableRequestFlags & VariableRequestFlag.GetVirtualKeys);
request.data.includesVirtualPath = !!(variableRequestFlags & VariableRequestFlag.VirtualPathIncluded);
request.data.threadIndex = smartBuffer.readUInt32LE(); // thread_index
request.data.stackFrameIndex = smartBuffer.readUInt32LE(); // stack_frame_index
const variablePathLength = smartBuffer.readUInt32LE(); // variable_path_len
Expand All @@ -50,7 +58,8 @@ export class VariablesRequest implements ProtocolRequest {
request.data.variablePathEntries.push({
name: protocolUtil.readStringNT(smartBuffer), // variable_path_entries - optional
//by default, all variable lookups are case SENSITIVE
forceCaseInsensitive: false
forceCaseInsensitive: false,
isVirtual: false
});
}

Expand All @@ -61,18 +70,28 @@ export class VariablesRequest implements ProtocolRequest {
request.data.variablePathEntries[i].forceCaseInsensitive = smartBuffer.readUInt8() === 0 ? false : true;
}
}

if (request.data.includesVirtualPath) {
for (let i = 0; i < variablePathLength; i++) {
//0 means case SENSITIVE lookup, 1 means forced case INsensitive lookup
chrisdp marked this conversation as resolved.
Show resolved Hide resolved
chrisdp marked this conversation as resolved.
Show resolved Hide resolved
request.data.variablePathEntries[i].isVirtual = smartBuffer.readUInt8() === 0 ? false : true;
}
}
}
});
return request;
}

public toBuffer(): Buffer {
const smartBuffer = new SmartBuffer();
const includesVirtualPath = this.data.variablePathEntries.some((entry) => entry.isVirtual);

//build the flags var
let variableRequestFlags = 0;
variableRequestFlags |= this.data.getChildKeys ? VariableRequestFlag.GetChildKeys : 0;
variableRequestFlags |= this.data.enableForceCaseInsensitivity ? VariableRequestFlag.CaseSensitivityOptions : 0;
variableRequestFlags |= this.data.getVirtualKeys ? VariableRequestFlag.GetVirtualKeys : 0;
variableRequestFlags |= includesVirtualPath ? VariableRequestFlag.VirtualPathIncluded : 0;

smartBuffer.writeUInt8(variableRequestFlags); // variable_request_flags
smartBuffer.writeUInt32LE(this.data.threadIndex); // thread_index
Expand All @@ -86,6 +105,13 @@ export class VariablesRequest implements ProtocolRequest {
//0 means case SENSITIVE lookup, 1 means force case INsensitive lookup
smartBuffer.writeUInt8(entry.forceCaseInsensitive !== true ? 0 : 1);
}

}

if (includesVirtualPath) {
for (const entry of this.data.variablePathEntries) {
smartBuffer.writeUInt8(entry.isVirtual !== true ? 0 : 1);
}
}

protocolUtil.insertCommonRequestFields(this, smartBuffer);
Expand All @@ -109,6 +135,16 @@ export class VariablesRequest implements ProtocolRequest {
*/
enableForceCaseInsensitivity: undefined as boolean,

/**
* Indicates whether the VARIABLES response should include virtual keys for the requested path
*/
getVirtualKeys: undefined as boolean,

/**
* Enables the client application to send path_is_virtual data
*/
includesVirtualPath: undefined as boolean,

chrisdp marked this conversation as resolved.
Show resolved Hide resolved
/**
* The index of the thread containing the variable.
*/
Expand All @@ -128,6 +164,7 @@ export class VariablesRequest implements ProtocolRequest {
variablePathEntries: undefined as Array<{
name: string;
forceCaseInsensitive: boolean;
isVirtual?: boolean;
}>,

//common props
Expand All @@ -138,6 +175,21 @@ export class VariablesRequest implements ProtocolRequest {
}

export enum VariableRequestFlag {
/**
* Indicates whether the VARIABLES response includes the child keys for container types like
* lists and associative arrays. If this is set to true (0x01), the VARIABLES response include the child keys.
*/
GetChildKeys = 1,
CaseSensitivityOptions = 2
/**
* Enables the client application to send path_force_case_insensitive data
*/
CaseSensitivityOptions = 2,
/**
* Indicates whether the VARIABLES response should include virtual keys for the requested path
*/
GetVirtualKeys = 4,
/**
* Enables the client application to send path_is_virtual data
*/
VirtualPathIncluded = 8
}
Loading
Loading