Acknowledge support for a list of adresses

[Gogny]

This PR is the first milestone for supporting acknowledgment and RTR frames. I called this feature activeACK to distinguish from the acknowledgement detection already present in the library.
The issue #18 is about the development of this feature.

The test results

There is a negligeable variation of the timing from frame to frame. My observations shows that the start of the ack can happen from 6.5us to 9us after EOD for a target of 7us, which I consider a good accuracy given no additional hardware timer is used. The end of the ack pulse is accurate within 1us, reproducing the behavior of an original equipment.
The function decisionActiveACK() can successful distinguish the frames with the correct addresses from the others, and do not triggers if it is a RTR frame or if the Request Acknowledge (RAK) bit is 0.

The raw sigrok/pulseview file is available (remove the .txt extension) :
activeACK timing 02 on cdc off.dsl.txt

[0xCAFEDECAF]

Thanks! I will review and test it.

[Gogny]

Hi ! Have you been able to review my code ? I am working on RTR frames and I would like to have this PR merged before creating another one :)

[0xCAFEDECAF]

Hi, I need to test on all supported hardware, not just ESP32-S3. So this takes some time, but I am quite busy atm.

I created a separate branch for you: RTR-frames. And I added you as collaborator. So you should be able to use this branch to merge to, and to add new features.

Please don't merge to master. I added a branch protection rule but I'm not sure if that prevents accidents...

[Gogny]

Thank you for your time. I understand the limits.
Developing new features on a dedicated branch is smoother, and when everything is final and tested you will be able to merge everything to master.
It is right we need to test this on WROOM, C3, C6, and ESP8266. I have devboards for these 4 but I will only be able to test with one IDEN (0x8EC) as this is the only equipment I have. Maybe testing with other ESP32s would be viable...

[Gogny]

About RTR I will clean my code and push it to the RTR branch at each step so you can give me your advice once you have time for that.
I would like to add support for CH32V006 microcontrollers once every feature I need are working. I will add the necessary compiler directives in local and only push them to the RTR branch if this uc can handle it despite running at 48MHz.

[0xCAFEDECAF]

Sounds like a good plan! I hope you can use the separate RTR branch to collect all your changes, which we can use to test on the various hardware platforms before merging into master.

Thanks, keep up the good work!

[0xCAFEDECAF]

Also: please make sure to run the workflows (compiler and linter tests). I think they run automatically on merge, but better to check before merging.

[Gogny]

I am not familiar with github workflows but I will get up to speed on these two.
I am in the process of coding a dedicated ISR for RTR replies based on SendBitIsr(). In the existing ISR, byte is of type uint16_t : uint16_t byte = *p_stuffedByte; (line 100 approx.).
As the ESP8266, ESP32 and CH32V006 are all 32 bits architectures, I figured out It may be more efficient to use uint32_t. My ISR prototype SendBitRtrIsr() works well with this approach. Is there a reason for storing the data on 16 bits instead of the full 32 bits ?

[0xCAFEDECAF]

Is there a reason for storing the data on 16 bits instead of the full 32 bits ?

No, not particularly. Feel free to experiment.

But please check your assumptions, i.e. if you expect a change to improve performance, do the test that proves that you are right. In this case I would add extra instrumentation to the ISR code (e.g. toggle a GPIO) so that the time spent in the ISR can be measured with a logic analyser. Measure before and after the assumed optimization.

[Gogny]

Hi ! Some work has been done since, and the RTR implementation is now complete, but not yet tested beyond my ESP32-S3. I used a 2nd timer because using the same for TX and RTR has been unsuccessful.

• 100% success rate (for now and with the S3), I always get an ACK
• detects if the RTR bit is LOW and aborts the RTR reply
• pulls the RTR bit LOW when starting the reply
• acknowledge detection available to the user via the VanBus.rtrAckReceived() function !

Moreover I have now a better understanding of the data structure used to communicate to and from the CD changer, most importantly the footer and header are iterative, every packet have the previous header +1.

Here are the code files, feel free to give me your feedback ;)
VanBusTx.cpp
VanBusRx.cpp
VanBus.h.txt
VanBusTx.h.txt
VanBusRx.h.txt

[0xCAFEDECAF]

That looks really cool! Indeed, using separate timers for separate purposes seems to be the way to go. Changing an existing timer (timerDetachInterrupt - timerAttachInterrupt) does not work well; it can work fine quite a few times but on the long run it does not work stable enough.

I experience the same with ESP8266, where the same timer is used for TX (SendBitIsr) and for RX ack timeout (WaitAckIsr). After running long enough, the TX timer ISR is no longer called, and there seems no way to get it back. This still needs to be fixed. Unfortunately, ESP8266 has only one HW timer available, so some code refactoring will be required.

I will have a look at it and test it thoroughly on the hardware I have available.