ARMmbed / DAPLink

https://daplink.io
Apache License 2.0
2.32k stars 979 forks source link

LED activity during WebUSB DAPJs flashing doesn't blink in 0254 #714

Closed microbit-carlos closed 6 months ago

microbit-carlos commented 4 years ago

Raising this issue here as the same version of DAPJs works fine with DAPLink 0253, but doesn't with 0254, but maybe this issue has to be transferred.

Basically, flashing with DAPJs via WebUSB is meant to blink the DAPLink LED, and this stopped working with DAPLink 0254.

Can be replicated using the DAPJs demo and a micro:bit board:

microbit-carlos commented 4 years ago

@thegecko tagging you to this ticket for visibility, as I am still not quite sure if this is a fix in DAPJs or DAPLink.

microbit-carlos commented 4 years ago

Quick note to add that serial activity via WebUSB doesn't blink the LED either in 0254.

thegecko commented 4 years ago

I agree this sounds like a DAPLink issue, with your repro steps it should really help tie down the change which introduced this regression

thegecko commented 4 years ago

Also worth mentioning we use the latest DAPjs with older DAPLink firmware and don't see this issue.

microbit-carlos commented 3 years ago

@flit a partial fix for this issue when WebUSB flashing was included as part of PR #763 in this commit: https://github.com/ARMmbed/DAPLink/pull/763/commits/e4e9b2d8b8fcc711fcfe9c4858701548643c825f

However, the LED still doesn't blink during "partial flashing". In the MakeCode and Python editor we use DAPJs with WebUSB CMSIS-DAP to manually erase/flash specific pages to update the user code only (JS code here). With older versions of DAPLink the LED would flash during partial flashing as well, but it currently doesn't.

flit commented 3 years ago

The issue is that the call to blink the "HID" LED is missing from the CMSIS-DAP v2 bulk interface handler.

https://github.com/ARMmbed/DAPLink/blob/78c51931f2058e72f5f95ec03c5299a6ff2bdde0/source/usb/bulk/usbd_bulk.c#L78-L84

microbit-carlos commented 6 months ago

This can be closed as fixed in https://github.com/ARMmbed/DAPLink/pull/1008 and refined in https://github.com/ARMmbed/DAPLink/pull/1019/