pybricksdev icon indicating copy to clipboard operation
pybricksdev copied to clipboard

EV3: Fix flashing on Windows.

Open jaguilar opened this issue 2 months ago • 6 comments

Windows requires a reportid byte at the front of each sent message for an HID device, even if the device doesn't use reportids. Adding this synthetic 0x00 to the front of the message enables successful EV3 flashing on Windows.

Tested by manually successfully flashing several times.

Fixes https://github.com/pybricks/support/issues/2391.

jaguilar avatar Dec 18 '25 05:12 jaguilar

Did a manual update here (Win11 25H2) and works. Thank you!

BertLindeman avatar Dec 18 '25 08:12 BertLindeman

This is great, thank you!

laurensvalk avatar Dec 18 '25 09:12 laurensvalk

Okay, for flashing with pybricksdev on Windows, it appears that doing this solves the problem:

if platform.system() == "Windows":
  # On Windows, HID reports require an extra leading byte for the
  # report ID, even if it's zero.
  message = b'\x00' + message

On read, we do not have to do the same, because of this code. Note that hid_write does not have any fixups to automatically add the reportid byte when it is needed, possibly because it cannot distinguish between a true leading zero of a message and a reportid? Anyway.

I'll send a pull request for pybricksdev.

Originally posted by @jaguilar in #2391

It looks like the Mac version is doing this in hid_write as well, so explicitly deleting 0x00 if it is there.

	if (report_id == 0x0) {
		/* Not using numbered Reports.
		   Don't send the report number. */
		data_to_send = data+1;
		length_to_send = length-1;
	}

Not entirely sure about the Linux version, but Linux is still working when I prepend 0x00 like you did.

Should we perhaps always prepend 0x00? See also the common spec for hid_write():

		/** @brief Write an Output report to a HID device.

			The first byte of @p data[] must contain the Report ID. For
			devices which only support a single report, this must be set
			to 0x0. The remaining bytes contain the report data. Since
			the Report ID is mandatory, calls to hid_write() will always
			contain one more byte than the report contains. For example,
			if a hid report is 16 bytes long, 17 bytes must be passed to
			hid_write(), the Report ID (or 0x0, for devices with a
			single report), followed by the report data (16 bytes). In
			this example, the length passed in would be 17.

			hid_write() will send the data on the first interrupt OUT 
			endpoint, if one exists. If it does not the behaviour is as 
			@ref hid_send_output_report

			@ingroup API
			@param dev A device handle returned from hid_open().
			@param data The data to send, including the report number as
				the first byte.
			@param length The length in bytes of the data to send.

			@returns
				This function returns the actual number of bytes written and
				-1 on error.
				Call hid_error(dev) to get the failure reason.
		*/
		int  HID_API_EXPORT HID_API_CALL hid_write(hid_device *dev, const unsigned char *data, size_t length);

laurensvalk avatar Dec 18 '25 10:12 laurensvalk

Prepending the zero byte does appear to work on Linux as well. Perhaps the kernel knows to add it if it's not present? It's weird, because that implies that it is not possible for the first byte of the message to be zero (it would be ambiguous if it were).

I cannot test on OS X since I don't have any Apple devices.

jaguilar avatar Dec 18 '25 15:12 jaguilar

Let's change this to always add the report ID in struct.pack(). I can test it on Mac later.

dlech avatar Dec 18 '25 15:12 dlech

Adjusted code per @dlech request. Verified that flashing still works on Windows and Linux.

jaguilar avatar Dec 18 '25 21:12 jaguilar

Just wanted to ping this. Anything more to be done or should we merge it?

jaguilar avatar Jan 15 '26 22:01 jaguilar

Thanks for the reminder. I forgot about this one. The code change looks good. We just need a changelog entry to go with it.

dlech avatar Jan 15 '26 22:01 dlech

Done.

jaguilar avatar Jan 16 '26 04:01 jaguilar

Thanks!

dlech avatar Jan 18 '26 22:01 dlech