rgrr / yapicoprobe

Yet Another Picoprobe
124 stars 12 forks source link

Added carriage return to end of debug prints #66

Closed tornupnegatives closed 1 year ago

tornupnegatives commented 1 year ago

Description

When using GNU screen, which does not automatically add carriage returns after new-lines, text wraps unnaturally across the console

For example:

  [CMSIS-DAPv1] [CMSIS-DAPv2] [MSC: DAPLink] [CDC: UART] [CDC: probe debug] [Net: 192.168.14.1 (NCM)] [Net: SysView] [Net: Echo] [Net: IPerf]
                                                             0.003 (  1) - (II) Probe HW:
         0.003 (  0) - (II)   Pico @ 240MHz
                                           0.003 (  0) - (II) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
                                                                      0.003 (  0) - (II) SWD clk req   : 2000kHz = 240000kHz / (6 * (20 + 0/256)), eff : 2000kHz
                                                                                0.056 ( 53) - (II) Assign tasks to certain cores
                                                0.000 (  0) - (II) =================================== MSC connect target
                                         0.114 (114) - (II) =================================== MSC disconnect target
                                     0.298 (184) - (II) =================================== MSC connect target
                              0.412 (114) - (II) =================================== MSC disconnect target
                          1.576 (999) - (II) =================================== MSC connect target
                   1.689 (113) - (II) =================================== MSC disconnect target

This commit addresses the issue by explicitly adding carriage returns at the end of all calls to picoprobe_info() and similar functions. I am not sure if this is generally useful, but it certainly will benefit me, so I am sharing just in case.

JimmyPedersen commented 1 year ago

Are you sure you got the newline in the correct place? IMHO it looks like it should be after the format parameter

rgrr commented 1 year ago

Interesting, never heard of screen before. What is your use case for it?

Concerning the output, xxd shows:

00000770: 7374 6f70 2e2e 2e0a 5b30 363a 3531 3a33  stop....[06:51:3
00000780: 322e 3338 3836 3038 2030 2e30 3030 3030  2.388608 0.00000
00000790: 325d 2030 2e30 3033 2028 2020 3329 202d  2] 0.003 (  3) -
000007a0: 2028 4949 2920 2b2b 2b2b 2b2b 2b2b 2b2b   (II) ++++++++++
000007b0: 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b  ++++++++++++++++
000007c0: 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b  ++++++++++++++++
000007d0: 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b  ++++++++++++++++
000007e0: 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b  ++++++++++++++++
000007f0: 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b 2b2b 0a5b  ++++++++++++++.[
00000800: 3036 3a35 313a 3332 2e33 3930 3233 3520  06:51:32.390235 
00000810: 302e 3030 3136 3239 5d20 302e 3030 3320  0.001629] 0.003 
00000820: 2820 2030 2920 2d20 2849 4929 2020 2020  (  0) - (II)    
00000830: 2020 2020 2020 2020 2020 2020 2020 2020                  
00000840: 2020 5765 6c63 6f6d 6520 746f 2059 6574    Welcome to Yet
00000850: 2041 6e6f 7468 6572 2050 6963 6f70 726f   Another Picopro
00000860: 6265 2076 312e 3136 2d31 3161 3361 3931  be v1.16-11a3a91
00000870: 0a5b 3036 3a35 313a 3332 2e33 3931 3430  .[06:51:32.39140
00000880: 3520 302e 3030 3131 3731 5d20 302e 3030  5 0.001171] 0.00
00000890: 3320 2820 2030 2920 2d20 2849 4929 2046  3 (  0) - (II) F
000008a0: 6561 7475 7265 733a 0a5b 3036 3a35 313a  eatures:.[06:51:
000008b0: 3332 2e33 3931 3830 3820 302e 3030 3034  32.391808 0.0004
000008c0: 3032 5d20 302e 3030 3320 2820 2030 2920  02] 0.003 (  0) 
000008d0: 2d20 2849 4929 2020 205b 434d 5349 532d  - (II)   [CMSIS-
000008e0: 4441 5076 315d 205b 434d 5349 532d 4441  DAPv1] [CMSIS-DA
000008f0: 5076 325d 205b 4d53 433a 2044 4150 4c69  Pv2] [MSC: DAPLi
00000900: 6e6b 5d20 5b43 4443 3a20 5541 5254 5d20  nk] [CDC: UART] 
00000910: 5b43 4443 3a20 7072 6f62 6520 6465 6275  [CDC: probe debu
00000920: 675d 205b 4e65 743a 2031 3932 2e31 3638  g] [Net: 192.168
00000930: 2e31 342e 3120 284e 434d 295d 205b 4e65  .14.1 (NCM)] [Ne
00000940: 743a 2053 7973 5669 6577 5d20 5b4e 6574  t: SysView] [Net
00000950: 3a20 4563 686f 5d20 5b4e 6574 3a20 4950  : Echo] [Net: IP
00000960: 6572 665d 0a5b 3036 3a35 313a 3332 2e33  erf].[06:51:32.3
00000970: 3933 3930 3120 302e 3030 3230 3934 5d20  93901 0.002094] 
00000980: 302e 3030 3320 2820 2030 2920 2d20 2849  0.003 (  0) - (I
00000990: 4929 2050 726f 6265 2048 573a 0a5b 3036  I) Probe HW:.[06
000009a0: 3a35 313a 3332 2e33 3934 3238 3420 302e  :51:32.394284 0.

which means that the probe is doing it's output correctly (or at least as intended): every line ends with a linefeed which is quite common in Linux world.

Perhaps your terminal program which you use in screen is broken?

I also agree with @JimmyPedersen that the carriage return is not placed correctly.

JustAnother1 commented 1 year ago

screen is a very useful Linux tool. It lets you keep processes running even when logged off. Use case is log in to a Linux box using ssh. start screen and start some script. The script runs and will take hours to complete. Exit screen, close ssh session. Do something valuable. Then after some hours log in using ssh again, open the screen session and you can see the script has finished. It can also handle the serial line.

I agree that it is normal for Linux text files to only have a \n to signal the next line. On terminal interfaces like the serial interface both \r and \n are needed. With \r only you can overwrite your text and therefore can create progress bars or update values without printing a new line with every update.

Therefore it makes sense to have \r and \n at the end of a line in a Linux terminal.

tornupnegatives commented 1 year ago

Are you sure you got the newline in the correct place? IMHO it looks like it should be after the format parameter

Good catch, thanks! I have moved the carriage return to the end of the format specifier, per my latest commit.

rgrr commented 1 year ago

if you introduce a cmake option for this feature (which defaults to off), I will merge this.

tornupnegatives commented 1 year ago

Done :-)

rgrr commented 1 year ago

Now it is fine. Thank you for your patience