Sidewinder 3-bit data reading is too slow

[necroware]

Some Sidewinder devices report the data in 3-bits per clock. Reading 3 times between the clocks is an extremely time critical task, which seem to fail for some devices. Unfortunately it is not possible to read all pins in one go, since they are all hanging on different MCU ports. That is due to historical reasons, since the adapter was initially designed for the simple analogue joysticks and at that point it was not clear how much more it will have to handle. Probably this issue could be fixed easier by redesign the hardware, but if it is possible to keep up the compatibility, I'd rather try to fix it in software.

[necroware]

First mentioned here:
#12 (comment)

[No0ne]

Thanks, now I understand.
Maybe this could also come handy regarding rewiring UART TX to the MIDI out pin?
(I can test this easily, I'm working on an arduino converted teensy v2 with just wires to a gameport connector)

Just discovered the serial debug feature:

Trying to enable digital mode
Guessing model by packet size of 0
Trying to enable digital mode
Guessing model by packet size of 0
Trying to enable digital mode
Guessing model by packet size of 0
Trying to enable digital mode
Guessing model by packet size of 11
Detected model 4
Detected device: MS ForceFeedBack Wheel
Packet decoding failed 1 time(s)
Packet decoding failed 2 time(s)
Packet decoding failed 3 time(s)
Packet decoding failed 4 time(s)
Packet decoding failed 5 time(s)
Packet decoding failed 6 time(s)
Sidewinder init...
Guessing model by packet size of 11
Detected model 4
Packet decoding failed 7 time(s)
Sidewinder init...
Guessing model by packet size of 11
Detected model 4
Packet decoding failed 8 time(s)
Sidewinder init...
Guessing model by packet size of 11
Detected model 4
Packet decoding failed 1 time(s)
Packet decoding failed 2 time(s)
Packet decoding failed 3 time(s)
Packet decoding failed 1 time(s)
Packet decoding failed 2 time(s)
Packet decoding failed 1 time(s)
Packet decoding failed 2 time(s)
Packet decoding failed 3 time(s)
Packet decoding failed 4 time(s)
Packet decoding failed 1 time(s)
Packet decoding failed 1 time(s)
Packet decoding failed 2 time(s)
Packet decoding failed 1 time(s)
Packet decoding failed 2 time(s)
Packet decoding failed 3 time(s)
Packet decoding failed 4 time(s)
Packet decoding failed 5 time(s)
Packet decoding failed 6 time(s)
Sidewinder init...
Guessing model by packet size of 11
Detected model 4
Packet decoding failed 1 time(s)
Packet decoding failed 1 time(s)
Packet decoding failed 2 time(s)
Packet decoding failed 1 time(s)
Packet decoding failed 2 time(s)
Packet decoding failed 3 time(s)

[necroware]

I had to fight this already for the Sidewinder Precision Pro. To be honest, I didn't think, that this will come up again :) The FFB Wheel implementation was contributed by another user. I have no such device at hand, so I couldn't test it myself, but as it was reported as working by multiple users, I assumed, that it's ok. Obviously not quite....

[caradas]

Here is my modified readPacket from Sidewinder.h
Sorry for the ugly magic numbers i planned to nice it up a bit before making it public but it should help for force feedback development.

   Packet readPacket() const {

      // Packet instantiation is a very expensive call, which zeros the memory.
      // The instantiation should therefore happen outside of the interrupt stopper
      // and before triggering the device. Otherwise the clock will come before
      // the packet was zeroed/instantiated.
      Packet packet;

      cooldown();

      // WARNING: Here starts the timing critical section
      const InterruptStopper interruptStopper;
      const auto ready = m_clock.isHigh();
      m_trigger.setHigh();

      // We are reading into a byte array instead of an uint64_t, because of two
      // reasons. First, bits packets can be larger, than 64 bits. We are actually
      // not interested in packets, which are larger than that, but may be in the
      // future we'd need to handle them as well. Second, for reading into an
      // uint64_t we would need to shift between the clock impulses, which is
      // impossible to do in time. Unfortunately this shift is extremely slow on
      // an Arduino and it's just faster to write into an array. One bit per byte.

      static const uint8_t wait_duration = 250;
      volatile char r1;
      volatile char r2;
      if (ready || m_clock.wait(Edge::rising, wait_duration)) {
        for (; packet.size < Packet::MAX_SIZE; packet.size++) {
          if (!m_clock.wait(Edge::rising, wait_duration)) {
            break;
          }
           
          //  Reading data bit by bit is to slow for 3 bit bus
          //  Originally when the 3rd bit was read it already shifted for the next clock cycle. 
          //  This reads the complete input register at once to filter it afterwards
          //  Also it only needs to read twice because data0 and d1 are in the same IO register. 
          //  From PinB register we need bit 4 as Button2/b1 and bit 3 as Button3/b2
          //  From PinE register we need bit 6 as Button4/b3
          //  See Atmega32u4 Documentation page 417 for explanation of addresses
          
          memcpy(&r1,0x23,1);
          memcpy(&r2,0x2c,1);

          //                              d1(pb4)           d1 (pb2)         d2 (pe6)
          packet.data[packet.size] = (r1 & 0x4)>> 1 | ((r1 & 0x10) >> 4) | ((r2 & 0x40) >> 4);
   
        }
      }
      
      m_trigger.setLow();
      return packet;
    }

[No0ne]

Jup, this fixes it completely!

[necroware]

Yes, it is the same idea, which I had, since two of the three bits are in the same register, so we actually need to read only twice. But I have to think about how to implement it in a clean way, since this is an absolutely evil hack :)

[necroware]

Hi guys! The idea to read the registers once and just get the bits out of it is great, but unfortunately that would either mean a messy code, which I prefer to avoid, or a massive rewrite of the registers reading code. I have a clue how to do that, but I'm currently quite short on time. So I decided to try something less complicated and optimized the code which is used to wait for the clock. The optimization is not huge, only couple of ticks, but that is may be enough to get this issue fixed. Would you please try out the code from the pull-request #21 with the wheel and see if it makes some difference?

Unfortunately all joysticks which I have at hand work flawlessly, so I can't validate this myself.

[No0ne]

git checkout 4e5c81255e7af782aab8c76239a49beb2d331b5b
My FFB Wheel works still fine with this!

[necroware]

Perfect! Thank you for testing. Then the optimization goes into the main branch.