search

nigelrogasch / MAGIC

Magnetic stimulator interface controller toolbox
23 stars 9 forks source link

Incorrect expected response length for 'Set Charge Delay' command #2

Open sjscymru opened 5 years ago

sjscymru commented 5 years ago

Hi there,

I came across this toolbox today, a colleague gave me a copy of paper published in the Brain Stimulation journal.

I actually work for Magstim and I've noticed an issue with the expected response length to the 'Set Charge Delay' (0x6E) command. This should be 6 and not 4 (line 414 of rapid.m). There is an error in the Magstim documentation, and the 'Set Charge Delay' command actually sends back a response in the same form as 'Get System Status' (0x78). That is, when setting the charge delay the response contains the command ack, instrument status, rapid status and the 2-byte extended instrument status + crc for a total of 6 bytes.

Kind Regards

nicolasmcnair commented 5 years ago

Hi there,

Thanks for getting in touch regarding our toolbox. We are definitely only getting back 4 bytes to the setChargeDelay command (command + instrument status + rapid status + CRC). However, the Rapid stimulator we have been testing on has software version 9. Is the 6 bytes something that's been introduced in version 11?

Also, the most recent communication protocol we're working off is revision 5.3 - which is I think valid up to software version 10? Is there a more up to date revision?

Cheers, Nick

On Tue., 12 Feb. 2019, 21:23 sjscymru <notifications@github.com wrote:

Hi there,

I came across this toolbox today, a colleague gave me a copy of paper published in the Brain Stimulation journal.

I actually work for Magstim and I've noticed an issue with the expected response length to the 'Set Charge Delay' (0x6E) command. This should be 6 and not 4 (line 414 of rapid.m). There is an error in the Magstim documentation, and the 'Set Charge Delay' command actually sends back a response in the same form as 'Get System Status' (0x78). That is, when setting the charge delay the response contains the command ack, instrument status, rapid status and the 2-byte extended instrument status + crc for a total of 6 bytes.

Kind Regards

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nigelrogasch/MAGIC/issues/2, or mute the thread https://github.com/notifications/unsubscribe-auth/ATS96qde7jyyc1N0BzidPFb7v2lpPUVyks5vMpYegaJpZM4a2EFa .

sjscymru commented 5 years ago

Hi Nick,

I've just had a look and yes, this change was made in V10.0 (which was never released) and thus is also present in V11.0. So V9.0 will return 4 bytes, V11.0 will return the 6 bytes.

The latest revision of protocol document is V5.3 which covers V11.0, but it contains this errata with reference to the number of bytes returned for the setChargeDelay function.

Kind Regards, Simon

nicolasmcnair commented 5 years ago

Ahh, thanks very much for that. We will updated the toolbox to fix this asap. Thanks again!

On Thu, 21 Feb 2019 at 03:37, sjscymru notifications@github.com wrote:

Hi Nick,

I've just had a look and yes, this change was made in V10.0 (which was never released) and thus is also present in V11.0. So V9.0 will return 4 bytes, V11.0 will return the 6 bytes.

The latest revision of protocol document is V5.3 which covers V11.0, but it contains this errata with reference to the number of bytes returned for the setChargeDelay function.

Kind Regards, Simon

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nigelrogasch/MAGIC/issues/2#issuecomment-465655739, or mute the thread https://github.com/notifications/unsubscribe-auth/ATS96sQAilTwjCKMxre0u5gVTQ-xoTClks5vPXmtgaJpZM4a2EFa .