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

Rebuilt rgbeffects file #40

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

MrPaulAR
Copy link

@MrPaulAR MrPaulAR commented Jan 6, 2022

I would suggest using this instead of the default rgbeffects file because when working in the sequencer there is much less lag. Also, there are quite a few groups that have been created which allow more effects such as the single line. This better aligns with how xLights was traditionally used on the house model.

The only sacrifice in this layout is that it breaks the sample show. If needed I can fix that too but a quick import will address that.

I would suggest using this instead of the default rgbeffects file because when working in the sequencer there is *much* less lag.  Also, there are quite a few groups that have been created which allow more effects such as the single line.  This better aligns with how xLights was traditionally used on the house model.

The only sacrifice in this layout is that it breaks the sample show.  If needed I can fix that too but a quick import will address that.
@JVB-Tesla
Copy link
Collaborator

model img

@MrPaulAR , this really speeds things up! Much appreciated.

It looks like there are some xLights channel mapping errors. If you can address these, we can merge. Would also appreciate you updating the example project.

@MrPaulAR
Copy link
Author

MrPaulAR commented Jan 7, 2022

I've went through the rgbeffects file and I'm not seeing any missing channels but I did add a new group for all the effects. If you see something specific please let me know and I will fix.

I also went through and imported the sample show that was provided in readme.md. In this I observed that one of your rear door handles was misnamed (rear rear door handle). I took the liberty of uploading this xsq & wav file into a Sequences/Sample folder. This will help keep things organized IMHO as all sequences will be under this folder structure.

├── Sequences
│   ├── Sample
│   │   ├── lightshow.wav # Audio file
│   │   ├── lightshow.xsq # xLights sequence
│   │   ├── lightshow.fseq # Sequence that is excluded from github and generated by xLights
│   │   ├── mapping.xmap # Mapping file ... see below
│   ├── User Sequence 1
│   │   ├── lightshow.wav # Audio file
│   │   ├── lightshow.xsq # xLights sequence
│   │   ├── lightshow.fseq # Sequence that is excluded from github and generated by xLights
│   ├── User Sequence 2
│   │   ├── lightshow.wav # Audio file
│   │   ├── lightshow.xsq # xLights sequence
│   │   ├── lightshow.fseq # Sequence that is excluded from github and generated by xLights
│   xlights_keybindings.xml # Not important in this context and not part of repo
│   xlights_networks.xml # Not important in this context and not part of repo
│   xlights_rgbeffects.xml

Lastly I included the mapping.xmap file in the Sequences/Sample folder. This can be used by anyone to import their previous version of xsq into this method. It's the same one I used to create the xsq in the Sample folder. As long as someone didn't create groups or something in the previous method it's a 30 second task to convert.

@adamcoulombe
Copy link

adamcoulombe commented Jan 7, 2022

Screenshot_31
These were the errors I got when I used the rgbeffects file. These might be the same channel mapping errors @JVB-Tesla is referring to.

@MrPaulAR I think you'll want to replace the rgbeffects file in the provided zip file too so that you dont have to manually copy it over.

One other thing I noticed that I'm not sure has to do with the new setup, when I try to reposition my house preview, it doesnt refresh or isnt responsive, is this just me? edit: It was just me, restarting xlights solved this

@MrPaulAR
Copy link
Author

MrPaulAR commented Jan 7, 2022

I fixed the layout errors by adding the xlights_networks.xml file. The error was due to the layout referencing a null controller by name for channel assignments as this is much quicker than clicking on a bunch of models individually.

Regarding the zip. I don't see any reason for it. As a matter of fact I just pushed a commit where I properly setup the xLights show folder based on xLights best practices based on their zoom room.

@MrPaulAR
Copy link
Author

MrPaulAR commented Jan 7, 2022

Additional comment on the zip. All but a couple images are inside that zip are just text files. IMHO we should let git version these.

@adamcoulombe
Copy link

Additional comment on the zip. All but a couple images are inside that zip are just text files. IMHO we should let git version these.

I tend to agree. I think they wanted to make it easy for those who are unfamiliar to download just the show folder.

I see you've removed the zip from your latest commit which might raise a separate issue; the readme still explains specifically to download that zip file:
https://github.com/MrPaulAR/light-show/blob/17cab69746b6173677d51616332d8cd5fb66492c/README.md?plain=1#L48

@MrPaulAR
Copy link
Author

MrPaulAR commented Jan 7, 2022

I don't think it is any easier to download a separate zip and extract it than to just download this one repo (clone or zip) but that is for the Tesla team to decide.

Good catch on the README.

Since my proposal is to have everything for xLights self contained without a zip I'm going to add a commit for that readme as well.

My new worry is that this has grown well past my original scope even though this is all directly related.

@adamcoulombe
Copy link

Yeah, agree. That was the only reason I kept the zip intact in my PR, was to keep the scope limited.

Anyway, I think you actually missed the reference in question though:

https://github.com/MrPaulAR/light-show/blob/e1e8b3d57f67b8cfee563b59d62ebfd30371c575/README.md?plain=1#L48

  1. Download and unzip tesla_xlights_show_folder.zip, which is the Tesla xLights bare project directory.

@MrPaulAR
Copy link
Author

MrPaulAR commented Jan 7, 2022

That appears to be in my current PR

image

@adamcoulombe
Copy link

That is line 77, the line in question is line 48 where it talks about show folder (not the example sequence)

@MrPaulAR
Copy link
Author

MrPaulAR commented Jan 7, 2022

I've added some additional verbiage to account for this. I've also removed a reference to the Model S xmodel since it's no longer one model but an entire layout.

Make path to Model S OBJ relative so that Xlights can find it.

I also restored the assets for the model S obj back in the show folder root since it is not exclusive to a sequence
@adamcoulombe
Copy link

Cool, nice work Paul.

I did test out your latest commit and found an issue with the model S no longer displaying, I sent note to you about this in a pull request for your patch-1 branch

Fix Model S obj not showing
@MrPaulAR
Copy link
Author

MrPaulAR commented Jan 7, 2022

Merged your pull. Thanks alot for doing the pull request. I'm currently just using web interface & github.dev for these as my stuff is currently in a private repo due to a bunch of sequences being there too.

@simon2022
Copy link

Thanks for your work, it's so much more fun to create lightshows without the lags of the program.
It's also much better with the groups, thank you!

@MrPaulAR could it be that the "Right Side Repeater" is not present in the 3D model?
lsr

Many greetings

@spaceXrace
Copy link
Contributor

spaceXrace commented May 7, 2022

Wow! Thank you so much. With nearly 20 lightshows programmed, I have been incredibly annoyed by the poor performance of XLights. This makes programming the shows so much faster and so much more fun.
I made people aware of your patches on r/TeslaLightShow, I hope you're ok with this!

@teslamotors teslamotors deleted a comment Jun 24, 2023
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.

5 participants