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

bme280 driver in Lua+C #3132

Merged
merged 1 commit into from
Oct 5, 2020
Merged

bme280 driver in Lua+C #3132

merged 1 commit into from
Oct 5, 2020

Conversation

vsky279
Copy link
Contributor

@vsky279 vsky279 commented May 29, 2020

Fixes #3126, #3124, #2241,

  • 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/*.

Rewritten BME280 driver based on the discussion in #3126.
I intend to add also SPI support (#2527) which should be also relatively small modification of the Lua driver.

There is a conflict when original BME280 module is included in the build. require('bme280') references to the original C module and not to the new Lua driver. This can be a bit confusing. The natural solution would be to that this driver replaces the original C driver.

I assume this should be probably merged only when #3128 is implemented.

Please review the proposed commit.

@vsky279 vsky279 changed the title bme280 drive in Lua+C bme280 driver in Lua+C May 29, 2020
@vsky279 vsky279 force-pushed the bme280neo branch 2 times, most recently from 425ad2e to 1ccdb8f Compare May 29, 2020 20:23
@vsky279
Copy link
Contributor Author

vsky279 commented May 30, 2020

I don't know how to make Travis CI happy when it complains that
lua_modules/bme280/bme280.lua:22:3: accessing undefined variable bme280_math
bme280_math is the new module so it should be known to the builder.

@galjonsfigur
Copy link
Member

@vsky279 You have to add entities in luacheck config file that can be found in tools/luacheck_config.lua. You can write it yourself or use luacheck config helper(also in tools folder) script that will generate this for you - little note on how to use it can be found here.

I thought that I've written documentation on how to add new entities in luacheck config when adding new C module but turns out I was wrong - I could add this in documentation or in project wiki.

@vsky279
Copy link
Contributor Author

vsky279 commented May 30, 2020

@galjonsfigur Thanks. I did manual update and now it's green.

@vsky279
Copy link
Contributor Author

vsky279 commented Jul 11, 2020

Lua 5.3 compatibility issue - lua_pushfstring works differently for non-printable characters

I was using lua_pushfstring with %c formating string to return string array of configuration parameters (3 bytes). Since Lua 5.3 the string pushed to stack for e.g. value 240 is <\240>. Different approach using just lua_pushstring is needed.

I am not sure if this cannot cause some issues in other modules.

luaO_pushfstring(L, "<\\%d>", cast_uchar(buff));

Copy link
Member

@nwf nwf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right! Yes, sorry to have forgotten about this one. I think it's worth landing and beginning to point people at it even if we don't yet quite have the one-shot "here's your firmware with both C and Lua modules in LFS baked in" story worked out in detail. (That depends on #3272 and probably #3138, and so probably won't be ready by next release, just given everyone's schedules, which is a little sad, but so it goes.)

Some minor changes requested.

app/include/user_modules.h Show resolved Hide resolved
docs/lua-modules/bme280.md Outdated Show resolved Hide resolved
mkdocs.yml Outdated Show resolved Hide resolved
mkdocs.yml Outdated Show resolved Hide resolved
@vsky279 vsky279 force-pushed the bme280neo branch 2 times, most recently from 63d7c18 to dfc4b62 Compare October 4, 2020 13:17
@nwf nwf merged commit b909178 into nodemcu:dev Oct 5, 2020
@vsky279 vsky279 deleted the bme280neo branch October 5, 2020 20:38
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.

4 participants