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

New spectrum color scheme #744

Merged
merged 1 commit into from Dec 17, 2022
Merged

Conversation

strijar
Copy link
Contributor

@strijar strijar commented Nov 9, 2022

I have implemented a new, more pleasing color scheme for the waterfall (like a sunset)

@Brumi-2021
Copy link
Contributor

Brumi-2021 commented Nov 10, 2022

Thanks 🙏
I will have a look as soon as merged in nightly.
Do you have some advanced picture about it ?

@gullradriel
Copy link
Member

Can't we go a bit further and implement a switch to allow the 2 color modes (and eventually more ?)

@strijar
Copy link
Contributor Author

strijar commented Nov 11, 2022

Yes, it's easy to do

@Brumi-2021
Copy link
Contributor

Brumi-2021 commented Dec 17, 2022

Hi , how about this forgotten nice PR ?
i think it is a nice proposal to change the LUT colour scheme of the waterfall.
Shall we also include it in “next” official merge or
do you prefer to keep both , the current and the new by user config ?
to me , I liked this one more than current , but how about your opinion ?
my vote , is to merge it , but let’s see more feedbacks .
4C071251-3EFC-4877-9CCA-559CC84FA6E2

@strijar strijar closed this Dec 17, 2022
@Brumi-2021
Copy link
Contributor

Brumi-2021 commented Dec 17, 2022

Hi @strijar ,
Sorry about our delay , but why did you close it ?
If you agree , we can reopen that PR and merge it , and it will be integrated to the next nightly and we may get more user feedbacks
Or are you working to make it, user config selection between both , current and your new LUT ?
Thanks !

@Brumi-2021 Brumi-2021 reopened this Dec 17, 2022
@Brumi-2021 Brumi-2021 merged commit 25c267a into portapack-mayhem:next Dec 17, 2022
@Brumi-2021
Copy link
Contributor

Hi again , let's merge it , and feel free to check that new Waterfall spectrum colour scheme in the new nightly firmware.
Based on @strijar , the color palette for the waterfall is only 768 bytes, so if wanted in future , we can add both (that one and the previous original one ) He, made this palette for UHSDR firmware.
Thanks a lot , @strijar , I like it very much !
Let's see other user feedback ,
Cheers,

@samukas81
Copy link

samukas81 commented Dec 17, 2022

Olá, é possível acrescentar a largura de banda de 100khz, ficaria mais preciso a leitura.

@Brumi-2021
Copy link
Contributor

Brumi-2021 commented Dec 17, 2022

Hello @samukas81 ,
but in that PR, we just changed the colour scheme , nothing about the BW coverage (largura de banda).
Then I feel that this comment is more a kind of "new issue" , than a feedback about that PR.
If you agree , you better open it in a new issue.

Cheers

@samukas81
Copy link

Olá obrigado por responder, não é um problema seria apenas uma implementação interessante!

@AlphaGeek1
Copy link

AlphaGeek1 commented Dec 18, 2022

I think it's always better to give users a choice rather than make choices for them, especially for subjective things like color palettes. I would like to see this as an optional choice in settings rather than something forced with no option.

@Brumi-2021
Copy link
Contributor

Hi thanks @AlphaGeek1 for your feedback.
Did you already check it in last nightly fw ?

I agree with you that is a subjective feature issue . But the developer launched it , one month ago , and as it is not related to any stability issue, we need to take some quick decision about it. In my personal opinion, i like it more than current one, and I prefer it . I feel that warm white and yellow colours give me a more clear image about too much RX signal , but it is just my opinion .
Anyway, do not worry , if we do not have a general consensus , we can always revert that PR , and wait for having selectable LUT scheme (as long as we do not have memory limitations) .
Feedback is always welcome ,

@jLynx
Copy link
Contributor

jLynx commented Dec 18, 2022

I agree, this should be an option and not forced

@Brumi-2021
Copy link
Contributor

  • Ok @jLynx , then , with your two opinions , is enough Clearly , we have different taste ,and it is too subjective. The important is the community feedback and decision , and give some answer to the developer. (If possible it will be better selectable , as it was pointed by @gullradriel )

@jLynx , you can revert it , no problem at all
(if not I will do it ) .
Cheers ,

@jLynx
Copy link
Contributor

jLynx commented Dec 18, 2022

Happy to keep it in as is for now. But if we could get another PR to make it as an option, that would be great

@jLynx
Copy link
Contributor

jLynx commented Dec 18, 2022

Actually having second thoughts now on weather it's even worth adding a option for it. Maybe it's fine as is. Anyone else have any strong opinions?

@samukas81
Copy link

samukas81 commented Dec 18, 2022

Seria legal poder escolher entre as duas cores, acredito não ser difícil fazer.

@eried
Copy link
Member

eried commented Dec 19, 2022

Actually having second thoughts now on weather it's even worth adding a option for it. Maybe it's fine as is. Anyone else have any strong opinions?

I also think it is ok with 1 scheme only. Unless users disagree

@samukas81
Copy link

Se não for muito complexo seria interessante ter duas opções, obrigado.

@AlphaGeek1
Copy link

AlphaGeek1 commented Dec 19, 2022

Why not make it so that the theme is an editable file on the SD card? That way, anyone with the time and desire could change it. Include the default sunset scheme and the original scheme (as another file). This way, it's settable (with the effort of changing/editing a file), the users have a choice not just between the two schemes because they could make their own if they choose, and there isn't an additional hit on limited memory.

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

7 participants