UVC Extension <--> Lepton Command ID off-by-one issue

[Sheyne]

In "usbd_uvc_if.c" in the static int8_t UVC_VC_ControlGet (VC_TERMINAL_ID entity_id, uint8_t cmd, uint8_t* pbuf, uint16_t length, uint16_t idx, uint16_t value) method, the logic for selecting the correct I2C command for a given UVC index is broken. It assumes that the LEP_CID_XXX macros ascend by multiples of 4. This is not always the case (see LEPTON_OEM.h, LEPTON_SYS.h, LEPTON_RAD.h). Suggest fixing by inserting a lookup table.

[Sheyne]

I have a fix written that I'll push on Monday. It's currently API breaking, so I'll have to tidy it up a bit first.

[kekiefer]

Thanks for looking after is, but I'm not sure there's a problem.

The documentation points out that in order to compute these register offsets you can take FLIR's register definitions and shift them over. What controls are you seeing where this approach doesn't work?

The registers might not increase by 4 bytes each time, but they increase by at least this much and always a multiple of this with just one exception (SYS AGC) which is handled as a one off. Are there others I missed?

The UVC specification considers that devices will not implement every standard control and the same mechanisms are in place for extension units. They do not have to be a contiguous register space.

You'll note that the UVC extension unit bitmasks are set up to indicate -- there are gaps in the control masks that should line up with the "missing" registers (but maybe I erred on some of these).

[Sheyne]

I know I was running into issues with the OEM LEP_COMMAND_IDs being off by one. I noted places where the register numbers skipped an index in the other headers, but I didn't test to see if I was getting off-by-one errors with them. Your solution with the bitmask probably works and you just missed that LEP_CID_OEM_MASTER_ID is commented out.

I have another solution with a lookup table for indices to "Module Command IDs" that is clearer, so we'll be less likely to end up with an off-by-one firmware bug, but it will change what indices map to. This will break any existing code that uses the UVC interface, so it might not be worth it.

Edit: here's the relevant commit: Sheyne/purethermal1-firmware@4c3e7886 . Note that the LEP_COMMAND_IDs are in sorted order rather than register order because that part of the code was generated by a script. We might want to switch that around.

[kekiefer]

How has your change been working out, did it solve the issues you were seeing?

[Sheyne]

It's been working perfectly and did solve my problem. I'm not sure how I feel about it from an engineering standpoint because it is brittle in the sense that it doesn't enforce the mapping between UVC integers and LEP_COMMAND_IDs, but the Lepton SDK is fixed so no new commands will be added. The best move might be to change the ordering I'm using to be as backwards-comparable as possible with the existing mapping in master.