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

RFD - Structuring the sensor modules #2243

Closed
devsaurus opened this issue Jan 30, 2018 · 2 comments
Closed

RFD - Structuring the sensor modules #2243

devsaurus opened this issue Jan 30, 2018 · 2 comments
Labels

Comments

@devsaurus
Copy link
Member

Introduction

We've lately seen a number of issues targeting the set of sensor modules which are embedded in the NodeMCU firmware. Requests to support variants of existing sensors (e.g. #2231, #2188) or reports of restrictions imposed by the current implementation (e.g. #2241). The suggestions and conclusions that came up on a per-case basis all pointed into a common direction in my opinion.

The challenge

Global view

The firmware contains several specialized modules which interface to external sensor or converter chips like ADXL345, BMP085, DS18B20, MCP4725, TCS34725 to name but a few. I expect that support for further chips or variants will increase the amount over time. This is not favorable IMO - I think that clustering / grouping would improve the overall view on the firmware/API from a user's perspective.

Module API architecture

Variants of existing chips that are similar but not the same triggered discussions on whether to fold them into existing API functions or set them up separately. I'm a fan of divide-and-conquer to keep the complexity of a given function/module low, but this would further drive the amount of specialized modules. Neither approach is favorable IMO and I assume this is a subtle obstacle for adding further features to the firmware.

Key learnings

The following principles are collected from the previous discussions and they could serve as hints for addressing the challenges above:

  1. Keep APIs lean, avoid covering different architectures, prefer focus on a specific chip.
    Applied in added support for ads1015 #2231.
  2. Cluster chips by functionality in "container-modules".
    Conclusion from QMC5883L support for hmc5883l.c module (#2187) #2188 to group support for QMC5883L and HMC5583L in a container.
  3. OOP techniques enable the points above, they should be the preferred solution (if possible and reasonable).

RFD

As initially stated, this all captures my personal take-aways and I'd like to ask for feedback & discussion of the aspects. My goal is to draft a guideline (finally transferred to the Wiki) which can be referenced in future issues and PRs.

@TerryE
Copy link
Collaborator

TerryE commented Mar 3, 2018

@devsaurus, Arnim, the main reason that I haven't reply is that at this level, I haven't got anything more to add. I support what you are saying. At the moment I feel that we've got the balance wrong somehow, and our approach to modules is just too anarchic. There is no consistency of APIs for similar classes of chip. The actual implementations are inconsistent in style quality, To many are one-off developments where the initial developer deposits the code and is not heard of again, yet because we have now incorporated it in our distro, the project is responsible for ongoing maintenance.

My current thinking is that we such impose a better structure on this.

  • Some modules are core to the esp chips and to the NodeMCU firmware and should be maintained by the project and incorporated in its standard distro. For these, at least, some of @Spiritdude comments in 2252 are very relevant: node, tmr, file, wifi, gpio, ow, ...
  • Others are H/W independent: cron, crypto, encoder, bit, mqtt, ...
  • And the last set are very chip-specific.

My view is that we should consider splitting this last group into a separate repository with its own style guide, FAQ, and contribution guidelines.

But the main problem that we have is developer resources to make this all happen, and in this we are currently struggling.

@stale
Copy link

stale bot commented Jul 21, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 21, 2019
@stale stale bot closed this as completed Aug 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants