search

SukkoPera / PsxNewLib

Playstation controller interface library for Arduino
GNU General Public License v3.0
133 stars 29 forks source link

Add support for the acknowledge pin #13

Open mthuurne opened 3 years ago

mthuurne commented 3 years ago

The acknowledge pulse can be very short: 4.5 μs on my 32u4 board, 3 μs on my PAL PSX. Therefore polling the pin risks missing the pulse. However, by using a masked external interrupt we can monitor the pulse reliably.

mthuurne commented 3 years ago

Is it OK to configure hardware in the constructor or should that be moved into begin()? And if so, should the argument also be moved or should that be stored by the constructor or made into a template argument?

Would it be better to add the interrupt flag technique to the DigitalIO library instead of in PsxNewLib itself?

SukkoPera commented 3 years ago

Please avoid arguments in c'tors. I think the ack pin can be made a template argument like the other pins. I think I did it that way in the devel branch.

The interrupt flag can be left in PsxNewLib. My only concern would be that using only pins capable of HW-Int is a bit limiting. I think I did it with pin-change interrupts in the devel branch but I'm not sure, it's been some time.

Anyway as long as it stays optional, it's a great feature that allows for more reliable communication, so thanks for your contribution. Let me know when it's complete.

mthuurne commented 3 years ago

Please avoid arguments in c'tors. I think the ack pin can be made a template argument like the other pins. I think I did it that way in the devel branch.

I'll rework the code.

I'm a bit surprised that NOT_A_PIN has value 0 rather than -1, since some boards do have a pin 0. Maybe I should be using -1 as the default value for the ack pin instead?

The interrupt flag can be left in PsxNewLib. My only concern would be that using only pins capable of HW-Int is a bit limiting. I think I did it with pin-change interrupts in the devel branch but I'm not sure, it's been some time.

I'm using a clone of the SparkFun Pro Micro, which is like the Arduino Leonardo. On this board external interrupts are available for pins 0, 1, 2, 3, 7 while pin change interrupts are available for pins 8, 9, 10 (as pin 11 and 17 are not accessible and pin 14, 15, 16 are in use by SPI). So external interrupts actually provide more options for this particular board.

mthuurne commented 3 years ago

I avoided the NOT_A_PIN issue by adding a subclass, similar to what you did on the devel branch.

Let me know when it's complete.

I think the implementation is complete now. Maybe the docs and examples need some changes so people will know the feature exists.

mthuurne commented 3 years ago

Would it be safe to wait for the falling edge of ACK instead of the rising edge? That would reduce latency a bit, especially if the pull-up on the ACK pin is weak.

SukkoPera commented 3 years ago

Pull-ups MUST be 1k. The library is not guaranteed to work with larger ones.

What I did in the devel branch is waiting for the falling edge of ACK and then for the line to get back high. If you just wait for a rising edge you can probably have less code and work just as well.

I would not just wait for the falling edge, that doesn't sound good to me. Latency is already much improved wrt not using the ACK pin at all. If you really wanted to spare that couple of microseconds, I guess you can introduce a new "fast but maybe unreliable" derivation of PsxController.

mthuurne commented 3 years ago

Pull-ups MUST be 1k. The library is not guaranteed to work with larger ones.

I already soldered 5k pull-ups, which seem to work for my board at least. If I ever do another one, I'll use 1k, but I don't want to risk breaking my working board by re-soldering.

The ACK pulse being 50% longer (4.5 μs instead of 3 μs) on my board is likely due to the 5k pull-up though. It was 15 μs when using only the internal pull-up of the 32u4.

What I did in the devel branch is waiting for the falling edge of ACK and then for the line to get back high. If you just wait for a rising edge you can probably have less code and work just as well.

Yes, waiting for the rising edge is what is currently implemented in this PR and that part required relatively little code. The biggest change I had to make was to make sure it doesn't wait for ACK after receiving the last byte.

I would not just wait for the falling edge, that doesn't sound good to me. Latency is already much improved wrt not using the ACK pin at all. If you really wanted to spare that couple of microseconds, I guess you can introduce a new "fast but maybe unreliable" derivation of PsxController.

If it's unreliable, it would not be worth saving a few microseconds. However, I don't know which flank of the ACK pulse is intended to be sampled: if the controller is ready at the falling flank, there would be no need to wait for the rising flank.