cs0x7f / cstimer

Professional Speedcubing/Training Timer
GNU General Public License v3.0
551 stars 132 forks source link

Improve WeiLong V10 Ai automatic MAC address discovery #409

Closed lukeburong closed 2 months ago

lukeburong commented 2 months ago

This should make automatic MAC address discovery work on all WeiLong V10 Ai smartcubes.

This method takes advantage of being able to perform memory reads via the Freqchip OTA update BLE service. We can read the MAC address from this struct. Unfortunately, the cube takes about 5 seconds to start responding normally again after reading the MAC address, meaning there is a delay after connecting to the cube - however, considering there was already a 10s timeout on manufacturer data-based MAC address discovery, this isn't really a big issue 😄

Tested and working for me on Chrome (Windows, Android) and Bluefy (iOS).

afedotov commented 2 months ago

This is very interesting approach! Is the OTA service allow to read any part of cube memory/firmware and this is common for such services? This may help for us in different task not related to MoYu cubes.

Regarding such implementation. I'm not sure how this is good and safe to use such approach, since there may be more cube models with the same protocol, and they may have different memory offsets. What do you think about approach I suggested in the previous PR? The cube command that needed to write any account id to V10 Ai cube is already known.

BTW reading Manufacturer Specific Data to obtain MAC address is common scenario used even in the original manufacturer's application.

lukeburong commented 2 months ago

Regarding such implementation. I'm not sure how this is good and safe to use such approach, since there may be more cube models with the same protocol, and they may have different memory offsets. What do you think about approach I suggested in the previous PR? The cube command that needed to write any account id to V10 Ai cube is already known.

I'm not a huge fan of that approach as:

  1. It still requires the MAC address to be found manually on the first connection as the bind account packet is encrypted.
  2. More importantly, it breaks compatibility with the official app - WCU CUBE won't allow you to connect to a cube with a non-zero account ID that doesn't match yours.

BTW reading Manufacturer Specific Data to obtain MAC address is common scenario used even in the original manufacturer's application.

Of course, and I would love to just use the manufacturer specific data, but we're unfortunately limited by the Chrome bug here 😆

I'm going to look into a more dynamic way of getting the jump table address (though I don't foresee it changing on this specific model unless they do it deliberately). The device name does contain the model too, so this could be gated behind the model name matching WCU_MY32 if another cube using the same protocol is released with an unknown offset and falling back to using manufacturer specific data if it doesn't.

afedotov commented 2 months ago

however, considering there was already a 10s timeout on manufacturer data-based MAC address discovery, this isn't really a big issue 😄

Actually this timeout is long lasts for 10s only in one single and rare case - the cube is stop sending advertising packets. Typically waitForAdvs() fails fast, in case if Web Bluetooth Advertising API is disabled it fails immediately, and also it proceed forward after first advertising packet is received no matter is there requested optionalManufacturerData with desired CIC exists. This timeout made bigger since some cubes like MoYu 2023 has very long advertising interval, and old short timeout of 5s was too small and sometimes fired before advertising packet from this cube is received and processed. All other cubes has much shorter advertising intervals.

afedotov commented 2 months ago

It still requires the MAC address to be found manually on the first connection as the bind account packet is encrypted.

Yep, indeed. We can try at first to use MAC address with known static prefix and device name, but this is not guaranteed that all cube instances and models in the future will have same MAC prefix and device name format.

More importantly, it breaks compatibility with the official app - WCU CUBE won't allow you to connect to a cube with a non-zero account ID that doesn't match yours.

Yes, same like you can't connect to different device/account, when you give your cube to friend for example. So user must unbind cube first with orange face turns, this is common scenario regardless of csTimer and must be covered in official cube manual.

Of course, and I would love to just use the manufacturer specific data, but we're unfortunately limited by the Chrome bug here 😆

This bug will be fixed eventually. But it takes time while all browsers in the wild will be updated to fresh version. In this case we can only guide users to use latest browser version.

lukeburong commented 2 months ago

Actually this timeout is long lasts for 10s only in one single and rare case - the cube is stop sending advertising packets

It will last for 10s if there is no suitable manufacturer data - the promise only resolves if getManufacturerDataBytes returns a value with byteLength >= 6. This is the case with CIC 0x0000. I did keep the timeout at 10s as I noticed the advertising packets received in Chrome were quite sporadic (despite the cube actually having an advertising interval of ~200ms) but I think it could probably be lowered to 5s. Still, in the case of an unbound cube (or cube with account ID <= 0xFFFF), it would be 5 seconds before prompting for MAC address.

This bug will be fixed eventually. But it takes time while all browsers in the wild will be updated to fresh version. In this case we can only guide users to use latest browser version.

Hopefully 🤞

afedotov commented 2 months ago

It will last for 10s if there is no suitable manufacturer data - the promise only resolves if getManufacturerDataBytes returns a value with byteLength >= 6.

Ah, so this may be simply fixed, if getManufacturerDataBytes returns undefined or less than 6 bytes, then immediately reject Promise.

IDK, actually both approaches seems awkward(( We can't guarantee that other cube models with same protocol will use same Freqchip SOC. Maybe keep this as is. Current behavior is almost usable:

And file bug to Chromium bug tracker, so this bug with 0x0000 CIC will be fixed eventually.

If only they extend API to allow any CICs, this will solve all the problems, but this is paranoid security decision I bet. Actually Bluefy does not follow specification here, and instead of Map returns plain DataView with all the bytes from Manufacturer Specific Data including CIC.

lukeburong commented 2 months ago

Otherwise we just get MAC from fixed prefix + device name.

I actually don't know whether it has a fixed prefix - that's just a remnant from when I was playing around with some of the code from the QiYi driver. I'd appreciate it if someone else with this cube could chime in here 😄

afedotov commented 2 months ago

This is how waitForAdvs() should be rewritten. And actually this also should be done in the GAN and QiYi cube drivers.

function waitForAdvs() {
    if (!_device || !_device.watchAdvertisements) {
        return Promise.reject(-1);
    }
    var abortController = new AbortController();
    return new Promise(function (resolve, reject) {
        var onAdvEvent = function (event) {
            DEBUG && console.log('[Moyu32Cube] receive adv event', event);
            var mfData = event.manufacturerData;
            var dataView = getManufacturerDataBytes(mfData);
            _device && _device.removeEventListener('advertisementreceived', onAdvEvent);
            abortController.abort();
            if (dataView && dataView.byteLength >= 6) {
                var mac = [];
                for (var i = 0; i < 6; i++) {
                    mac.push((dataView.getUint8(dataView.byteLength - i - 1) + 0x100).toString(16).slice(1));
                }
                resolve(mac.join(':'));
            } else {
                reject(-3);
            }
        };
        _device.addEventListener('advertisementreceived', onAdvEvent);
        _device.watchAdvertisements({ signal: abortController.signal });
        setTimeout(function () { // reject if no mac found
            _device && _device.removeEventListener('advertisementreceived', onAdvEvent);
            abortController.abort();
            reject(-2);
        }, 10000);
    });
}
cs0x7f commented 2 months ago

I think the risk of reading and writing OTA interface is too high. It is difficult for us to guarantee that such operation is safe enough, especially on uncontrollable hardware. It is difficult for us to guarantee whether the current instruction will become other functions in the future (for example, the current read operation may be a formatting operation in the future, which is uncontrollable). Unless necessary, csTimer should not have additional impact on hardware. For example, many hardware provide interfaces for resetting the cube state, but csTimer will choose the software reset method (multiply the inverse of the "solved" state) instead of resetting the cube state in the hardware, simply because the latter is more dangerous.

lukeburong commented 2 months ago

I think the risk of reading and writing OTA interface is too high. It is difficult for us to guarantee that such operation is safe enough, especially on uncontrollable hardware. It is difficult for us to guarantee whether the current instruction will become other functions in the future (for example, the current read operation may be a formatting operation in the future, which is uncontrollable).

Fair enough.

Unless necessary, csTimer should not have additional impact on hardware. For example, many hardware provide interfaces for resetting the cube state, but csTimer will choose the software reset method (multiply the inverse of the "solved" state) instead of resetting the cube state in the hardware, simply because the latter is more dangerous.

This is also why I didn't like the approach of binding a fake account ID to the cube - not a huge fan of "writing" to the cube and also breaking compatibility with the official app without "unbinding" the cube with the orange face.

Current MAC address discovery method seems like the best we'll be able to achieve until Chrome fixes the 0x0000 CIC bug 😄