apache / nuttx-apps

Apache NuttX Apps is a collection of tools, shells, network utilities, libraries, interpreters and can be used with the NuttX RTOS
https://nuttx.apache.org/
Apache License 2.0
268 stars 504 forks source link

examples/hx711: add new program to test hx711 chip #2317

Closed mlyszczek closed 5 months ago

mlyszczek commented 5 months ago

new example program to text hx711 chip

xiaoxiang781216 commented 5 months ago

@mlyszczek please fix the follow warning:

ERROR __main__.py:618: Check failed: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/hx711/CMakeLists.txt
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/hx711/hx711_main.c:79:109: error: Long line found
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/hx711/hx711_main.c:80:108: error: Long line found
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/hx711/hx711_main.c:81:79: error: Long line found
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/hx711/hx711_main.c:82:92: error: Long line found
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/hx711/hx711_main.c:84:95: error: Long line found
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/hx711/hx711_main.c:85:97: error: Long line found
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/hx711/hx711_main.c:86:95: error: Long line found
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/hx711/hx711_main.c:88:80: error: Long line found
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/hx711/hx711_main.c:89:83: error: Long line found
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/examples/hx711/hx711_main.c:91:99: error: Long line found
mlyszczek commented 5 months ago

These are strings. Aren't strings exempted from this check? I wanted to remove indentation at first, but then checker complains about no indentation.

xiaoxiang781216 commented 5 months ago

These are strings. Aren't strings exempted from this check? I wanted to remove indentation at first, but then checker complains about no indentation.

you can split to two short string, for example:

"\t-t<prec> tares the scale with specified precision,"
"might take few seconds to complete.\n"
mlyszczek commented 5 months ago

I think it looks kinda bad, it's harder to see how output is going to be printed and is hard to grep later on. Even Linux allows for long strings without breaking.

How about I write patch to nxstyle to ignore such cases? I think it would be enough to simply check if line ends with an " then in that case ignore line length warning?

acassis commented 5 months ago

I think it looks kinda bad, it's harder to see how output is going to be printed and is hard to grep later on. Even Linux allows for long strings without breaking.

How about I write patch to nxstyle to ignore such cases? I think it would be enough to simply check if line ends with an " then in that case ignore line length warning?

@mlyszczek I think it is happening on Linux because they increased the column from 80 to 120. If you download an old kernel (i.e. 2.6 series) you will see they enforce 80 column limit event for strings.

mlyszczek commented 5 months ago

I am all game for 80ch line width. Shorter lines are way easier to read than longer. Hell, I think newspapers keep it down at 40 characters or so, their columns are always so thin.

But I am against sticking to some arbitrary rule 100% of time. Especially if that rule in specific case hinders readability. These lines are still < 80ch, because I didn't want long lines in terminal output, but since these are in switch statement they are quite far right. I would opt in to remove indentation for these strings altogether, but that is going to break another rule (easy to fix, if first character is " then ignore indent).

I'm not gonna argue here really, just showing my point of view. I will break these lines if you wish, no problem, really. I just think, this rule should be lifted for strings.

And as a bonus some Linux Kernel documentation where 80 still seem to be preferred line width.

The preferred limit on the length of a single line is 80 columns.

Statements longer than 80 columns should be broken into sensible chunks,
**unless exceeding 80 columns significantly increases readability and
does not hide information.**

(...)

However, never break user-visible strings such as printk messages
because that breaks the ability to grep for them.
jerpelea commented 5 months ago

+1 for 80 characters limit

mlyszczek commented 5 months ago

Very well then. Should be fixed now.

It's definitely harder to read, if you ask me.