search

CMUAbstract / tab

TAB reference implementations, examples, and documentation
Other
0 stars 0 forks source link

Add support for per-command LED toggle to the CDH example #20

Closed mablem8 closed 1 year ago

mablem8 commented 1 year ago

In the TAB loop, toggle the LEDs (one lit, the other dark) each time the CDH board sends a reply. You will need to leverage something similar to this termination condition in the LED toggle code.

AnandAlok0807 commented 1 year ago

Commit 86fe85d is intermediate work for this issue in the sprint-3_alok branch.

Testing is in progress for implementation (currently with internal clock once tested, will include HSE clock) :

  1. Toggle the LEDs with CDH board TAB response implemented in cdh_monolithic.c, at line here
  2. header inclusion in cdh.h at line
AnandAlok0807 commented 1 year ago

Commit efd90bf is intermediate work for this issue in the sprint-3_alok branch.

Toggles LED at every formulation of TAB response, as TX buffer has no state to monitor and after transmit (data is pooped and buffered is empty ( also a normal state), makes the choice to toggle LEDs as soon as reply is formulated for transmission.

Testing done with implementation (currently with internal clock once tested, will include HSE clock) :

Toggle the LEDs with CDH board TAB response implemented in cdh_monolithic.c, at line here

Terminal results :

p3-env) abstract@lab:~/git-repos/tab-sprint-3/python-examples/tx$ python3 tx_example.py /dev/ttyUSB0 txcmd: common_ack hw_id:0x1234 msg_id:0x0000 src:gnd dst:cdh reply: common_ack hw_id:0x1234 msg_id:0x0000 src:cdh dst:gnd

txcmd: common_nack hw_id:0x1234 msg_id:0x0001 src:gnd dst:cdh reply: common_nack hw_id:0x1234 msg_id:0x0001 src:cdh dst:gnd

txcmd: common_debug hw_id:0x1234 msg_id:0x0002 src:gnd dst:cdh "Hello, world!" reply: common_debug hw_id:0x1234 msg_id:0x0002 src:cdh dst:gnd "Hello, world!"

txcmd: common_data hw_id:0x1234 msg_id:0x0003 src:gnd dst:cdh 0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a 0x0b reply: common_ack hw_id:0x1234 msg_id:0x0003 src:cdh dst:gnd

txcmd: common_data hw_id:0x1234 msg_id:0x0004 src:gnd dst:cdh 0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x0a 0x09 0x0b reply: common_nack hw_id:0x1234 msg_id:0x0004 src:cdh dst:gnd

txcmd: bootloader_ack hw_id:0x1234 msg_id:0x0005 src:gnd dst:cdh reply: common_nack hw_id:0x1234 msg_id:0x0005 src:cdh dst:gnd

txcmd: bootloader_nack hw_id:0x1234 msg_id:0x0006 src:gnd dst:cdh reply: common_nack hw_id:0x1234 msg_id:0x0006 src:cdh dst:gnd

txcmd: bootloader_ping hw_id:0x1234 msg_id:0x0007 src:gnd dst:cdh reply: bootloader_ack hw_id:0x1234 msg_id:0x0007 src:cdh dst:gnd reason:0x00(pong)

txcmd: bootloader_erase hw_id:0x1234 msg_id:0x0008 src:gnd dst:cdh reply: bootloader_ack hw_id:0x1234 msg_id:0x0008 src:cdh dst:gnd reason:0x01(erased)

mablem8 commented 1 year ago

The fix looks good, but it also includes code aimed at addressing Issue #2. Consolidate the fix into a single commit independent from code aimed at addressing other issues and post the commit hash here.

AnandAlok0807 commented 1 year ago

Commit 113bfc0 is intermediate work for this issue in the sprint-4-issue-20 branch.

Toggles LED at every formulation of TAB response, as TX buffer has no state to monitor and after transmission (data is popped and buffered is empty (also a normal state), makes the choice to toggle LEDs as soon as a reply is formulated for transmission.

Testing is performed with implementation (currently with an internal clock once tested, will include HSE clock) : Toggle the LEDs with CDH board TAB response implemented in cdh_monolithic.c, at line 26-27 and line 38-41

Terminal output result : p3-env) abstract@lab:~/git-repos/tab-sprint-3/python-examples/tx$ python3 tx_example.py /dev/ttyUSB0 txcmd: common_ack hw_id:0x1234 msg_id:0x0000 src:gnd dst:cdh reply: common_ack hw_id:0x1234 msg_id:0x0000 src:cdh dst:gnd

txcmd: common_nack hw_id:0x1234 msg_id:0x0001 src:gnd dst:cdh reply: common_nack hw_id:0x1234 msg_id:0x0001 src:cdh dst:gnd

txcmd: common_debug hw_id:0x1234 msg_id:0x0002 src:gnd dst:cdh "Hello, world!" reply: common_debug hw_id:0x1234 msg_id:0x0002 src:cdh dst:gnd "Hello, world!"

txcmd: common_data hw_id:0x1234 msg_id:0x0003 src:gnd dst:cdh 0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a 0x0b reply: common_ack hw_id:0x1234 msg_id:0x0003 src:cdh dst:gnd

txcmd: common_data hw_id:0x1234 msg_id:0x0004 src:gnd dst:cdh 0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x0a 0x09 0x0b reply: common_nack hw_id:0x1234 msg_id:0x0004 src:cdh dst:gnd

txcmd: bootloader_ack hw_id:0x1234 msg_id:0x0005 src:gnd dst:cdh reply: common_nack hw_id:0x1234 msg_id:0x0005 src:cdh dst:gnd

txcmd: bootloader_nack hw_id:0x1234 msg_id:0x0006 src:gnd dst:cdh reply: common_nack hw_id:0x1234 msg_id:0x0006 src:cdh dst:gnd

txcmd: bootloader_ping hw_id:0x1234 msg_id:0x0007 src:gnd dst:cdh reply: bootloader_ack hw_id:0x1234 msg_id:0x0007 src:cdh dst:gnd reason:0x00(pong)

txcmd: bootloader_erase hw_id:0x1234 msg_id:0x0008 src:gnd dst:cdh reply: bootloader_ack hw_id:0x1234 msg_id:0x0008 src:cdh dst:gnd reason:0x01(erased)

mablem8 commented 1 year ago

The solution compiles, and the solution works with the test script. I recommend moving the if(tx_cmd_buff_o->empty) { block after the line here.

mablem8 commented 1 year ago

To be a little more clear, I recommend moving the location of the toggle code because, although it works with the test script, your proposed location of the toggle code has a serious bug.

Suppose a command has been received by the CDH board, and a reply has been constructed. Upon transition from the reply construction to the reply transmission, the LEDs toggle in commit 113bfc0280e5b9ae3a7c8a7b93d2697429441b83. However, if tx_usart1 exits before transmitting the entire reply, then the LEDs will toggle back before the remainder of the reply can be transmitted.

This bug can occur if, for example, the UART baud rate is slow compared to the MCU clock speed. From an observer’s perspective, it will likely appear as if the LEDs never toggled at all.

By toggling the LEDs inside of the tx_usart1 function after the while loop and conditioning on tx_cmd_buff_o->empty as suggested, the LEDs toggle only when transmission ends due to an empty TX command buffer (and not when transmission pauses due to a full UART TX buffer), which is the desired behavior.

AnandAlok0807 commented 1 year ago

Commit c1cc48c fixes issue in the sprint-4-issue-20 branch.

Toggle the LEDs with CDH board TAB response implemented in cdh.c at line 228-230.

Setting initial LED state in cdh-monolithic.c, at line 27

--> LED toggle is implemented inside of "tx_usart1" function inside of while loop as the default state is also empty) and with the condition "tx_cmd_buff_o->empty". LED toggle inside tx_usart1 after TX buffer is empty, confirming successful TAB CMD transmission.

mablem8 commented 1 year ago

The solution compiles. Testing shows that the solution works. Resolution accepted and issue closed.