Add device.getDirectory

[commandblockguy]

Adds a method to get the list of variables on a device.

This PR also adds support for receiving fragmented virtual packets and an expectAny method for receiving a virtual packet of unknown type.

It also makes several changes to parameter handling:

• Adds a public function for getting parameters from the device
• Allows non-numerical parameters to be constructed and destructed
• Fixes a bug in which an unrecognized parameter caused parsing to fail, as the code always expected a size to be provided, despite this not being the case when the device doesn't return a value for the parameter.
• Fixed a test case which relied on this bug
• Changed the return type of destructParameters to be an object rather than an array and to only include valid parameters
• Updated the test cases to use the new return format

I've only tested this on the CE, so it will probably need additional testing before being merged.

[Timendus]

Hey there! Another great looking PR at first glance, thanks a lot! I'll have a better look this weekend, when I hope to have a little spare time and access to my calculator. One thing that jumps out to me is the dropped test coverage. It would be nice to have a unit test for the mergeBuffers method and a replay for the getDirectory. I can make the latter, can you maybe do the former?

[commandblockguy]

I've added a test for mergeBuffers. It didn't affect the coverage percentage, since it's already used by another function, but it did find a bug, so I guess it served its purpose?

[commandblockguy]

I've added a recording and tests for the TI-83 Premium CE, which work fine if you mark the device as "supported" and completely fail otherwise. I'm not really sure how to fix that, as the required support level isn't explicitly specified anywhere during the test.

[Timendus]

Alright, I made the build and CodeClimate (and myself 😉) happy again.

However, there's this annoyance in the test output a couple of times now:

console.log
    Warning: Unlogged read of property 'vendorId', value: 1105

      at Player._expectProperty (src/webusb/player.js:72:17)
          at Array.every (<anonymous>)
          at Array.find (<anonymous>)

  console.log
    Warning: Unlogged read of property 'productId', value: 57352

      at Player._expectProperty (src/webusb/player.js:72:17)
          at Array.every (<anonymous>)
          at Array.find (<anonymous>)

  console.log
    Warning: Unlogged read of property 'productName', value: TI-83 Premium CE

      at Player._expectProperty (src/webusb/player.js:72:17)
          at Array.every (<anonymous>)
          at Array.find (<anonymous>)

I'm not entirely sure why that didn't show up before, when I just changed the support type of the TI-83 Premium CE to supported. It's not really a huge issue I suppose. But if you have any ideas... 😄

If you're still happy after my changes, I'll merge the PR.

[Timendus]

Oh, and by the way, it works perfectly on my TI-84 Plus 👍🏻

[commandblockguy]

I'm also seeing those warnings - I'm guessing it's because changing the support level caused it to check if the device was supported again? I'll investigate it later today, and if necessary, re-record the tests.

[Timendus]

Tbh, I don't think it's an issue with the recording. There shouldn't really be something like a second check for different support levels. Not sure what it is though.

So maybe this shouldn't be a blocking issue. I guess it's a good idea if I add a getDirectory recording for TI-84+ though, and we should add this to the readme file, to round off this PR.

[Timendus]

Found anything? I myself am a bit pressed for time last week and this week, but I'll attempt to find a moment to make the TI-84+ recording and add it to the PR.

[commandblockguy]

I believe the issue is here. The properties required for device matching are checked once for each device for the current support level, so changing the support level changed the number of devices and caused the properties to be read more times than before. So, this is only a testing issue, not an issue with the actual code. The simplest solution would simply be to create a copy of the device parameters before comparing to each device. This would also speed up recognizing devices when there are many supported calculators.

[Timendus]

You're a genius. That could very well be the issue 🎉

[SpaceSaver]

It would be nice if this support was added.