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

Virtual Hid Mini feedback #73

Closed
wants to merge 1 commit into from
Closed

Virtual Hid Mini feedback #73

wants to merge 1 commit into from

Conversation

hansmbakker
Copy link

@hansmbakker hansmbakker commented Jul 23, 2019

@samtertzakian I created this PR to keep track of my feedback.

HID definition refactoring

I think the HID definitions can be shared between the HID mini module and the VHF module (f9fdf98)

Timer code in generic module

I'm not sure the timer code is needed in the generic Dmf_VirtualHidDeviceMini module. The timer is used to simulate incoming data, and other drivers would have their own data generation.

From the sample intro documentation:

It also creates a periodic timer to check the queue and complete any pending request with data from the device. Here timer expiring is used to simulate a hardware event that new data is ready.
...
Later when data is ready (as simulated by timer expiring), the driver checks for any pending request in the manual queue, and then completes it.

Personally I would move that timer logic to the sample module, and only have a function like GenerateInputReport that roughly looks like below in the generic Dmf_VirtualHidDeviceMini module. That function can then probably be called from the sample module?
If both the timer logic (to cleanup outstanding queue requests?) and a manual report generation function are needed, then it is my misunderstanding.

void
GenerateInputReport(
	_In_  WDFQUEUE          ManualQueue,
	_In_  MY_INPUT_DATA inputData
)
{
	NTSTATUS                status;
	PMANUAL_QUEUE_CONTEXT   queueContext;
	WDFREQUEST              request;
	MY_INPUT_REPORT	readReport;

	KdPrint(("GenerateInputReport\n"));

	queueContext = GetManualQueueContext(ManualQueue);

	//
	// see if we have a request in manual queue
	//
	status = WdfIoQueueRetrieveNextRequest(
		queueContext->Queue,
		&request);

	if (NT_SUCCESS(status)) {
		readReport.ReportId = MY_REPORT_ID;
		readReport.Data = inputData;
		
		status = RequestCopyFromBuffer(request,
			&readReport,
			sizeof(readReport));

		WdfRequestComplete(request, status);
	}
}

@hansmbakker hansmbakker changed the title Feedback Virtual Hid Mini feedback Jul 23, 2019
@samtertzakian
Copy link
Member

Hi, Hans,

I would like to make this change in a different way. I agree we need common definitions, but I would like to do it differently than the way you did it in this PR.

I have noted all your feedback and I agree (including about the timer). I will try to make an update for all your feedback within a day or so.

@hansmbakker
Copy link
Author

hansmbakker commented Jul 25, 2019

Hi Sam,
that's clear, then I'm curious how you would implement it :)

@hansmbakker
Copy link
Author

Closing this PR since feedback is discussed in #69.

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.

None yet

2 participants