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

[ServiceInfo] Do not embed color in python code #2256

Closed
wants to merge 1 commit into from

Conversation

nautilus7
Copy link
Contributor

@nautilus7 nautilus7 commented Sep 7, 2019

In service info, when pressing the green button, the ecm info is showing in yellow/orange color. This is because the color in embedded in the python code. So, instead of setting a color in the code, we should allow skins to determine what color to use.

After this change, skins will continue to work perfectly fine and the ecm info will be displayed in whatever color is already selected for the service info screen.

Instead, let skins to determine what color to use.
@Dima73
Copy link
Contributor

Dima73 commented Sep 8, 2019

What for?
Do you want to force to rewrite all skins?
Example:
green instead of light green

@nautilus7
Copy link
Contributor Author

No, you are totally wrong!

The color in the "ecm info" tab will now be the same as in the "pids info" tab. And this color is what is defined for the "infolist" in the skin.

Read the python code before commenting the next time. Thanks.

@Dima73
Copy link
Contributor

Dima73 commented Sep 8, 2019

@nautilus7
Copy link
Contributor Author

nautilus7 commented Sep 8, 2019

I don't understand what you mean by this...

Whatever color is selected by the skin for the self["infolist"] is going to be used for ecm info as well.
Do you have any clue which exactly are the screens I am talking about?
Did you bother checking the code?
Did you bother checking how your current skin works with my change?

@Dima73
Copy link
Contributor

Dima73 commented Sep 8, 2019

What is incomprehensible here?
If you add color to the "infolist", the all list will be of this color.

@nautilus7
Copy link
Contributor Author

nautilus7 commented Sep 8, 2019

Even if a skin does not set a color for infolist explicitly, it gets a default color like every skin element.

After my change all possible infolist elements like "pids", "ecm info" and "tuner values" will have that same color defined in the skin.

Why do we need to force all skins to have the ecm info displayed in orange color? There is no reason for that. So remove the hard-coded color from the python code!

@littlesat
Copy link
Member

Can you post some screenshots so I can see what’s meant here?

@littlesat
Copy link
Member

When you want the skin decides the color or overrules the default do something with attributes.

@nautilus7
Copy link
Contributor Author

nautilus7 commented Sep 8, 2019

Go to menu --> information --> service info

There you will see the text in whatever color your skin has selected.
Then press the green button to display the "ecm info". The text will be in yellow color BECAUSE this color is defined in the python code!

See how it displays right now:
1_0_19_7531_426_1_C00000_0_0_0_20190909013714
1_0_19_7531_426_1_C00000_0_0_0_20190909013722

The skin cannot change the yellow color in the ecm info!
My change will allow the color to be the same as in the other screen (white).

@Dima73
Copy link
Contributor

Dima73 commented Sep 9, 2019

This two color
1_0_16_70A_12_70_1680000_0_0_0_20190909071536

@nautilus7
Copy link
Contributor Author

OK, I'll add an attribute to the code, so it can change colors

@littlesat
Copy link
Member

Indeed the skin cannot change the color code that highlights the current 'chosen' ECM. So this change request makes no sense. I agree it should be skinnable. But then we need something to arrange with parameters/attributes that forward the color to python.

@littlesat littlesat closed this Sep 9, 2019
@nautilus7
Copy link
Contributor Author

Believe it or not I had never notice that multiple ecm pids could be listed in that screen. I always thought that only the active pid is displayed there. Now I understand the reason behind the hard-coded color.

@nautilus7
Copy link
Contributor Author

@littlesat I can make it read a color from the skin instead of using a hard-coded color from the python.

But, I think it would be better to indicate the active ecm pid using a word (e.g. "active") instead of using a different color. It looks much simpler. I don't see why a different color is needed.

What do you think???

1_0_1_C1D_1E78_71_820000_0_0_0_20190909220715

@nautilus7 nautilus7 deleted the ecm-info-color-fix branch September 20, 2019 17:56
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.

None yet

3 participants