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

Introduce Garland module #2408

Merged
merged 61 commits into from
Dec 24, 2020
Merged

Introduce Garland module #2408

merged 61 commits into from
Dec 24, 2020

Conversation

ElderJoy
Copy link
Contributor

@ElderJoy ElderJoy commented Dec 12, 2020

Port of https://github.com/Vasil-Pahomov/ArWs2812 from Arduino to ESP8266
Implementing garland of WS2812
Features for now:

  • 8 animation modes
  • Web control of ON/OFF and brightness
  • env:garland setup

In plans:

  • add more animations
  • add more controls to web

It is quite raw but works for me on wemos-d1mini. Hope with community help it can be useful. New Year is coming... :)

ElderJoy and others added 22 commits January 25, 2020 01:02
# Conflicts:
#	code/espurna/data/index.all.html.gz
#	code/espurna/data/index.light.html.gz
#	code/espurna/data/index.lightfox.html.gz
#	code/espurna/data/index.rfbridge.html.gz
#	code/espurna/data/index.rfm69.html.gz
#	code/espurna/data/index.sensor.html.gz
#	code/espurna/data/index.small.html.gz
#	code/espurna/data/index.thermostat.html.gz
#	code/espurna/espurna.ino
#	code/espurna/static/index.all.html.gz.h
#	code/espurna/static/index.light.html.gz.h
#	code/espurna/static/index.lightfox.html.gz.h
#	code/espurna/static/index.rfbridge.html.gz.h
#	code/espurna/static/index.rfm69.html.gz.h
#	code/espurna/static/index.sensor.html.gz.h
#	code/espurna/static/index.small.html.gz.h
#	code/espurna/static/index.thermostat.html.gz.h
#	code/html/index.html
#	code/platformio.ini
@ElderJoy
Copy link
Contributor Author

Check failed on env:esp8266-4m-latest-base. Checked locally, build for env:esp8266-4m-latest-base failed with error:
espurna\config/hardware.h:5078:6: error: #error "UNSUPPORTED HARDWARE!!"
Does anybody know if it is my fault? What's wrong and what can I do to fix this issue?

code/espurna/garland/scene.h Outdated Show resolved Hide resolved
@mcspr
Copy link
Collaborator

mcspr commented Dec 12, 2020

Also note it merges every .cpp file into one when building in CI. static / namespace { ... } might still have name collisions. I also noticed end-of-file line-endings are missing for some of the .cpp files.

Do you think it is possible to move Anim code into a library? i.e. you can just use a separate repo ElderJoy/GarlandESP8266 or something like that

@mcspr
Copy link
Collaborator

mcspr commented Dec 12, 2020

Checked locally, build for env:esp8266-4m-latest-base failed with error:
espurna\config/hardware.h:5078:6: error: #error "UNSUPPORTED HARDWARE!!"

...I missed the question about local builds. You can check out the travis log above the error, since it's just a shell script and it shows every step it does. The -base environments are mission DEVICE and MANUFACTURER pair of build flags somehow, either custom.h, src_build_flags of the custom env, ESPURNA_FLAGS=... environ, PLATFORMIO_SRC_BUILD_FLAGS=... environ, etc.

You can try adding test/build/garland.h with the GARLAND_SUPPORT=1 (+ any additional flags), so the CI can try to build it

@ElderJoy
Copy link
Contributor Author

Hi @mcspr,
Thank you for pointing out my mistakes. I fixed slashes, add end-of-file where it was missed and wrap code that belongs to garland into macro to exclude it from build in other configurations.

As for separate garland-dedicated code into separate library. I supposed this functionality as extendable. Anyone can easily add and modify animations. Hope with community help we will have much more animations soon.
It would be not so easy with external library. But it is possible of cause.
So, should I separate this code in library?

@ElderJoy
Copy link
Contributor Author

The -base environments are mission DEVICE and MANUFACTURER pair of build flags somehow, either custom.h, src_build_flags of the custom env, ESPURNA_FLAGS=... environ, PLATFORMIO_SRC_BUILD_FLAGS=... environ, etc.

Yes, I noticed that base envs missed that flags, but not clearly understand how supposed to build them in this way.

You can try adding test/build/garland.h with the GARLAND_SUPPORT=1 (+ any additional flags), so the CI can try to build it

I added env:garland with correspondent flags. Ok, will add configuration for CI also.

@mcspr
Copy link
Collaborator

mcspr commented Dec 12, 2020

See https://github.com/xoseperez/espurna/wiki/PlatformIO#building
The idea is to preserve the build environment when changing build flags. Locally or in CI, effectively it skips the framework + libs build, only rebuilding the relevant project source

As for separate garland-dedicated code into separate library. I supposed this functionality as extendable. Anyone can easily add and modify animations. Hope with community help we will have much more animations soon.
It would be not so easy with external library. But it is possible of cause.
So, should I separate this code in library?

Let me get back on that...

code/espurna/garland/anim_assemble.cpp Outdated Show resolved Hide resolved
code/espurna/garland/anim_comets.cpp Outdated Show resolved Hide resolved
@ElderJoy
Copy link
Contributor Author

Ok, next attempt. Fixed all pointed places.
New features:

  • possibility to control common animation speed from UI
  • reset brightness and speed to the default
  • each animation can tune self speed factor
  • calculate average frame rate for animation and output with debug info
    Tune some animations
    Please take a look on it now.

Copy link
Collaborator

@mcspr mcspr left a comment

Choose a reason for hiding this comment

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

Something have been bothering me... sorry that I missed this when initially reading the description

Original project does not specify any license. And, based on the Github's explanation from here:
https://docs.github.com/en/free-pro-team@latest/github/creating-cloning-and-archiving-repositories/licensing-a-repository#choosing-the-right-license

However, without a license, the default copyright laws apply, meaning that you retain all rights to your source code and no one may reproduce, distribute, or create derivative works from your work. If you're creating an open source project, we strongly encourage you to include an open source license.
...
If you publish your source code in a public repository on GitHub, according to the Terms of Service, other users of GitHub have the right to view and fork your repository.

It is not really clear if with the changes here it is 'derivative work' or something else. We are not commercially distributed software though, so there's that. But, I wonder if we could at least make the author aware of the inclusion to receive an ok that we can use it?
(there is also a continued work, as the project deemed deprecated, but it still lacks any license there as well)

if (!_cache[i].empty())
return _cache[i];

Color col = getPalColor((float)i / 256);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was 256 meant to be cache size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. And it takes 256*3 bytes per palette. But dramatically reduce processing time. And some animations use this actively.
Parameter (byte i) has 256 possible variants. So, there are 256 variants of interpolated colors for the palette doesn't matter how many different static colors it has. Any animation that produce smooth color sequence for palette use this for each pixel each Run cycle.

code/espurna/garland/scene.h Show resolved Hide resolved
@ElderJoy
Copy link
Contributor Author

ElderJoy commented Dec 21, 2020

It is not really clear if with the changes here it is 'derivative work' or something else. We are not commercially distributed software though, so there's that. But, I wonder if we could at least make the author aware of the inclusion to receive an ok that we can use it?
(there is also a continued work, as the project deemed deprecated, but it still lacks any license there as well)

Yes, you're right. It is better to contact to Author of library that I used as base one. Unfortunately, I didn't found any contact information neither in GitHub account nor in the code. So, I found a bit weird way - I created issue in the currently maintained project. Hope Author will contact to me or write here.

@ElderJoy
Copy link
Contributor Author

@mcspr Ok, what we can do more regarding License? Version that I used is deprecated now (I used Arduino version) and I rewrite it mostly. But of cause base version was that I pointed out and most animations from there. So, can we use it based on GitHub licensing politics?

@mcspr
Copy link
Collaborator

mcspr commented Dec 23, 2020

It may be a stretch, but assuming we follow the chain to the https://github.com/bportaluri/ALA, it is GPLv3 still even without an explicit statement that it is.
I'd add attribution as 'inspired / based on the code from ... and ...' to the files though.

edit: Github docs are only meant to point out that the author holds the rights, but does not tell us what our rights are and what we could do with the code (besides just looking at it)

@ElderJoy
Copy link
Contributor Author

Added header with reference to the project garland based on

@mcspr mcspr merged commit 5aeb24f into xoseperez:dev Dec 24, 2020
@mcspr
Copy link
Collaborator

mcspr commented Dec 24, 2020

Just in time :) sry for delays

I also wanted to point out that Adafruit's neopixel lib might not be the most efficient one on the platform, you might want to check out either:

and esp8266 code there was not really updated since august of 2015
https://github.com/adafruit/Adafruit_NeoPixel/commits/master/esp8266.c

@mcspr
Copy link
Collaborator

mcspr commented Dec 24, 2020

Could you update the wiki describing the module?

@ElderJoy
Copy link
Contributor Author

Well, it works not so bad with Adafruit's Neopixel currently. However, it eat most time for showing pixels inside library and if it can be speed up, than it worth to do. So, sure, I will check both mentioned libs and adapt library to better one. But my proposition to make it with next pull request. I going to add couple animations also.

@ElderJoy
Copy link
Contributor Author

Could you update the wiki describing the module?

Yep, will do.

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