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

BMEXXX sensor documentation update (Lua 5.3 compatibility) #3149

Closed
wants to merge 1 commit into from

Conversation

vsky279
Copy link
Contributor

@vsky279 vsky279 commented Jun 7, 2020

Fixes - no reported issue>.

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/*.

BME280 and BME680 sensor modules documentation includes examples that are

  • not Lua 5.3 compatible (("%d"):format(3/2) fails as 3/2 has no integer representation)
  • were intented for integer version so includes code that is not needed in now standard flow version of the firmware.

@pjsg
Copy link
Member

pjsg commented Jun 8, 2020

Again, I'm not convinced that you should be removing compatibility with some versions of the firmware.

@vsky279
Copy link
Contributor Author

vsky279 commented Jun 8, 2020

I don't like the current code not being compatible with Lua 5.3. Another way to make it compatible but also working in integer version is to add math.floor, i.e.

local Tsgn = (T < 0 and -1 or 1); T = Tsgn*T
print(("T=%s%d.%02d"):format(Tsgn<0 and "-" or "", math.floor(T/100), T%100))

instead of just

print(("T=%.2f"):format(T/100))

Do we need such complexity in an example?
I tend to think that if someone goes for the integer version, he/she has to build it himself, so I assume he/she knows what he's doing, knowing all the implications.

Anyway in the new BME280 driver (#3132) returned values are float so it is not integer version compatible. Should it be?

@marcelstoer
Copy link
Member

The examples in the docs should work with whatever defaults we have in the firmware (5.1 vs. 5.3). If an example only works with int or float then it should mention that clearly.

@vsky279
Copy link
Contributor Author

vsky279 commented Sep 4, 2020

This PR has
+ readability of the example and it's simplicity
- one version for float another for integer version

It seems that consent is that compatibility is winning over simplicity (simply said) so I will close this PR.

@vsky279 vsky279 closed this Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants