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

Can't have one FW that supports S31 (and Pow2) and Displays #5106

Closed
Frogmore42 opened this issue Feb 4, 2019 · 3 comments
Closed

Can't have one FW that supports S31 (and Pow2) and Displays #5106

Frogmore42 opened this issue Feb 4, 2019 · 3 comments
Labels
question Type - Asking for Information

Comments

@Frogmore42
Copy link
Contributor

Yes, it does not make sense to have display support on an S31, since that would be unsafe for any usage. But, I like to use the same FW image on multiple/all of my devices. I have some that have displays and the S31s do not. With the current setup, it is impossible to configure one FW image to meet all my needs.

Describe the solution you'd like
A configurable option to allow having Displays and Energy Monitoring

Describe alternatives you've considered
the other option is to create one image for energy monitoring and another for everything else, which is workable.

Better documentation on this space saving choice would be good. It took me a couple of hours to figure it out.

@arendst
Copy link
Owner

arendst commented Feb 4, 2019

Be my guest and roll your own. As you already noticed it's all about memory usage, flash usage and equal I2C addresses. In my_user_config.h you find some figures which may give an indication for your choices.

I've made the current choices based on general use and hopefully not to many issues raised due to lack of memory (See TLS related issues related to memory issues too).

My local user_config_override.h contains a lot of disabled features but I still need at least two versions to satisfy the display drivers while I want them to be below 500k (which I prefer).

EDIT: I see no reason why you cannot have both energy monitoring AND displays. Just enable them in user_config_override.h

@Frogmore42
Copy link
Contributor Author

Frogmore42 commented Feb 4, 2019

The problem is sonoff_post.h. When you select USE_SENSORS and USE_DISPLAYS, the latter wins, and disables all the sensors. Probably a simple check for USE_SENSORS before turning off stuff might be good, but I don't know your intention.

I definitely agree the default shouldn't have everything and making two flavors of the FW is not too hard, but PlatformIO doesn't really make it easy.

EDIT: I see I can use the individual settings to enable displays. If the intention of USE_DISPLAYS is really a larger image contents definition that is also going to delete other stuff, what you have is great. I would probably have called it IMAGE_DISPLAYS and created bundles of settings for major features that require multiple defines to get everything. So USE_DISPLAYS would set USE_DISPLAY and all the other display defines.

It was a learning experience for me and I was forced to learn more about how PlatformIO works and I got the download tool from espressif, which is must faster than the way PlatformIO and gives better feedback.

@ascillato2 ascillato2 added the question Type - Asking for Information label Feb 4, 2019
@arendst
Copy link
Owner

arendst commented Feb 5, 2019

I guess you found out how to use it.

Your idea of changing USE_DISPLAYS to IMAGE_DISPLAYS is good but it will provide issues for people used to their my_user_config.h and platformio.ini settings.

Thx for your input.

@arendst arendst closed this as completed Feb 5, 2019
arendst added a commit that referenced this issue Feb 8, 2019
6.4.1.15 20190208
 * Change image name BE_MINIMAL to FIRMWARE_MINIMAL (#5106)
 * Change image names USE_xyz to FIRMWARE_xyz (#5106)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Type - Asking for Information
Projects
None yet
Development

No branches or pull requests

3 participants