Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 2, 2024

Conversation

mlyszczek
Copy link
Contributor

new example program to text hx711 chip

@xiaoxiang781216
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

acassis commented Feb 28, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

+1 for 80 characters limit

@mlyszczek mlyszczek force-pushed the add-hx711-example branch 2 times, most recently from cb5bbfb to 0d3de5e Compare February 29, 2024 19:45
@mlyszczek
Copy link
Contributor Author

mlyszczek commented Feb 29, 2024

Very well then. Should be fixed now.

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

Signed-off-by: Michał Łyszczek <michal.lyszczek@bofc.pl>
@xiaoxiang781216 xiaoxiang781216 merged commit 8669861 into apache:master Mar 2, 2024
26 checks passed
@mlyszczek mlyszczek deleted the add-hx711-example branch March 2, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants