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

Improve error handling for TPS546 i2c communications #427

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mutatrum
Copy link
Contributor

My compiler complained about unused variables in the variables used in the read functions. When I tried to fix it, I noticed on a lot of places there is no error handling, so I tried to improve it a bit. My assumption is that the i2c communications shouldn't fail halfway, and if it fails, something is wrong. With this change it'll abort operations.

I'm not 100% sure if this is correct and I don't have a BitAxe with a TPS546, so this is a bit of a stab in the dark. It is more in line with how the error handling for other i2c components are done.

Write functions have the same issue, where the esp_err_t return value is not checked, even with multiple sequential writes. I assume this needs to have the same treatment?

@mutatrum
Copy link
Contributor Author

mutatrum commented Oct 24, 2024

These were the compiler errors. I can set them to warning, but I think having them break the compiler is a good thing.

/home/mutatrum/code-workspace/esp-miner/main/TPS546.c: In function 'TPS546_get_frequency':
/home/mutatrum/code-workspace/esp-miner/main/TPS546.c:576:12: error: 'value' may be used uninitialized [-Werror=maybe-uninitialized]
  576 |     freq = slinear11_2_int(value);
      |            ^~~~~~~~~~~~~~~~~~~~~~
/home/mutatrum/code-workspace/esp-miner/main/TPS546.c:572:14: note: 'value' was declared here
  572 |     uint16_t value;
      |              ^~~~~

This is caused by setting CONFIG_COMPILER_OPTIMIZATION_PERF (-O2) instead of CONFIG_COMPILER_OPTIMIZATION_DEBUG (-Og). This is the only file it complains about.

@skot
Copy link
Owner

skot commented Nov 30, 2024

after working on #524 I see what you mean about inconsistent error handling. We should really be pushing esp_err_t all the way up and printing helpful output to the log. We just had a user the other day with i2c problems, but the log didn;t say which device was causing it.

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.

2 participants