Replies: 8 comments 4 replies
-
The effect will work in mesmerizer with the API key. the Pull request 564 has a link where you can get the free API key that will allow it to run successfully. As also mentioned in the pull request there is more functionality that is desired before it is merged into the main branch. You can disable the effect trhought the board's web gui, or if you comment out line 359 in src/effects.cpp it will not be included in the build , |
Beta Was this translation helpful? Give feedback.
-
I'm admittedly not paying a lot of attention to the tree/lists right now,
but if you dont care about stocks, why would you be on the Marshall's
unmerged stocks branch?
I mean, yeah, there needs to be a way to turn it off and once we're all
ready to focus on that, I'm sure there'll be an easy "off" button - not
accidentally adding stock symbols and not accidentally signing up for an
API key and entering into your device being easy hints - but it's entirely
possible that code under development just has that wired to be turned on
on. When I was working on the matrix effects branch, I certainly turned ALL
my effects ... and only my effects ... on just for developer testing
convenience. I wouldn't be totally shocked if Marshall's done the same.
"Doctor, Doctor, it hurts when I do this." :-)
…On Fri, Dec 22, 2023 at 11:18 PM Marshall G Gates ***@***.***> wrote:
The effect will work in mesmerizer with the API key. the Pull request 564
<#564> has a
link where you can get the free API key that will allow it to run
successfully. As also mentioned in the pull request there is more
functionality that is desired before it is merged into the main branch.
You can disable the effect trhought the board's web gui, or if you comment
out line 359 in src/effects.cpp it will not be included in the build ,
—
Reply to this email directly, view it on GitHub
<#572 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD376DQOYJZKA6HP7D53YKZSRPAVCNFSM6AAAAABBALEBISVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM3TSMZSGQYDG>
.
You are receiving this because you are subscribed to this thread.Message
ID:
<PlummersSoftwareLLC/NightDriverStrip/repo-discussions/572/comments/7932403
@github.com>
|
Beta Was this translation helpful? Give feedback.
-
This is actually why I don't like adding new variables to secrets(.example).h, because it breaks cases where someone has a secrets.h without the new variable and moves to a source code version that requires it. In hindsight it would probably have been better to make the secrets defines instead of actual variables; that would have allowed us to issue more useful build-time errors if one of them isn't defined. Anyway, that's a case of "de koe in de kont kijken" in Dutch - which I won't translate but expresses the fact that it isn't very useful. |
Beta Was this translation helpful? Give feedback.
-
I'll take some blame for not correctly understanding the problem report,
but Harrie, part of the problem was this report.
"erizer, but get an error message because I didn't fill in the stock..."
I thought this was a *runtime* error.
It was a *build* error.
Harrie, when you make a report, please be specific. The words of the error
message - and details like whether you see them on the screen of the
mesmerizer, on the serial console of the ESP-32, or on your Real Computer
when you attempt to build it - matter a lot.
I gave a bad answer because I incorrectly guessed this was an error message
on the device itself and not a compile time error. If the text of the error
had been copy-pasted into the report, it would have saved us all a bunch of
time.
Rutger said:
In hindsight it would probably have been better to make the secrets
defines instead of actual variables; that would have allowed us to issue
more useful build-time errors
They are. We COULD issue better errors; we just don't. Exhibit A:
include/deviceconfig.h
68: String location = cszLocation;
include/secrets.h
37:#define cszLocation "12345"
We can trivially add a block to deviceconfig.cpp that takes the form
#if !defined(cszLocation)
#error Missing czzLocation definition from include/secrets.h
#endif
.... repeated for each of the additional extra seven defines.
IMO, it's a bit of copy-paste work to cover for a small developer edge
case, but if you think it's worthwhile, I'll throw a change like that into
the oven.
Maybe include/deviceconfg.h also gets something like
#if !__has_include("secrets.h")
#error Copy include/secrets-example.h to include/secrets.h, edit to taste,
and retry. Please see README.md#getting-started-with-the-source-code
#endif
That seems like an undeserved number of words, though.
I can take a stab at the lifting, but am concerned that developer-proofing
the builds can build in undeserved brittleness. (If we rearrange the
README, who is going to think to edit a source file? Approximately
nobody....)
Rutger, if you'd like, I can take a stab to
1. add #if !defined(BLAH) ... times eight - and then knowing that
they're going to get out of sync over time.
2. add better error for just plain not reading the doc and making a secrets
file to start with
There's a less verbose form of (1) we could do. I'll offer it separately:
1a)
#if !defined(a) || !defined (b) || !defined(c) ...
# error Your include/secrets.h is out of date. Compare secrets.h and
secrets.example.h
#endif
Advantages: Don't have to provide an error for all #defines. Doesn't have a
wall of text checking for an extremely specific case.
Disadvantages: Doesn't provide a unique error for all #defines.
FWIW, I don't feel strongly about implementing any of these. I think
they're fairly low-value changes that will ultimately be a long-term
maintenance hassle, but I'm probably not the audience we'd be trying to
help with this kind of thing.
Rutger, step up to the window and place your order for some reasonable
combination of 1, 1a, and 2 and I'll take a swing at implementing it and
send a PR your way.
RJL
|
Beta Was this translation helpful? Give feedback.
-
On Sun, Dec 24, 2023 at 2:52 AM Rutger van Bergen ***@***.***> wrote:
They are. We COULD issue better errors; we just don't.
I'll be damned, you're right (of course).
This is why we get paid the big bucks.
that I didn't even bother to check.
I had the file open for another silly reason or I wouldn't have, either. :-)
There's this thing they say about people who assume...
You and I undertake these big brain things on this tiny stuff exactly to
avoid that from happening.
But in that case, and combining a number of elements in your suggestions,
I think we could do the following:
That's pretty much exactly options 1 and 2, only in deviceconfig.h instead
of deviceconfig.cpp...where it would probably never fail because some other
compilation would fail first. [ sadtrombone.com ] Rats. OK, Good
modification.
- They're not used in DeviceConfig, but it's almost Christmas.
Close enough that I may not actually get back to it before it technically
IS (or is after) but it's the thought that counts.
- Add an item to the "six step guide to adding a device setting"
that's already at the top of deviceconfig.h, to mention the addition of a
define check if a default in secrets.h is introduced.
Good one.
Does that sound somewhat reasonable?
Sold.
It's a lot of bytes (that ultimately disappear) to carry around for a
pretty silly problem that doesn't happen very often at all.
Still, it removes some exposed electrical wiring that's USUALLY not fatal,
so I'll tuck some of that in...
Soon. I think I'm done with this day. More company arrives in a few hours
and sleep needs to be between now and them.
RJL
… Message ID:
<PlummersSoftwareLLC/NightDriverStrip/repo-discussions/572/comments/7937116
@github.com>
|
Beta Was this translation helpful? Give feedback.
-
Continued in
#574
Consider any changes you want to make to that pre-approved and I think we
have the "hard" part already in agreement. I'll probably be offline for a
while...
…On Sun, Dec 24, 2023 at 3:40 AM Robert Lipe ***@***.***> wrote:
On Sun, Dec 24, 2023 at 2:52 AM Rutger van Bergen <
***@***.***> wrote:
> They are. We COULD issue better errors; we just don't.
>
> I'll be damned, you're right (of course).
>
This is why we get paid the big bucks.
> that I didn't even bother to check.
>
I had the file open for another silly reason or I wouldn't have, either.
:-)
There's this thing they say about people who assume...
>
You and I undertake these big brain things on this tiny stuff exactly to
avoid that from happening.
> But in that case, and combining a number of elements in your suggestions,
> I think we could do the following:
>
That's pretty much exactly options 1 and 2, only in deviceconfig.h
instead of deviceconfig.cpp...where it would probably never fail because
some other compilation would fail first. [ sadtrombone.com ] Rats. OK,
Good modification.
>
> - They're not used in DeviceConfig, but it's almost Christmas.
>
> Close enough that I may not actually get back to it before it technically
IS (or is after) but it's the thought that counts.
>
> - Add an item to the "six step guide to adding a device setting"
> that's already at the top of deviceconfig.h, to mention the addition of a
> define check if a default in secrets.h is introduced.
>
> Good one.
>
>
> Does that sound somewhat reasonable?
>
Sold.
It's a lot of bytes (that ultimately disappear) to carry around for a
pretty silly problem that doesn't happen very often at all.
Still, it removes some exposed electrical wiring that's USUALLY not fatal,
so I'll tuck some of that in...
Soon. I think I'm done with this day. More company arrives in a few hours
and sleep needs to be between now and them.
RJL
> Message ID:
> <PlummersSoftwareLLC/NightDriverStrip/repo-discussions/572/comments/7937116
> @github.com>
>
|
Beta Was this translation helpful? Give feedback.
-
I updated the pull request it the necessary tests for the stock ticker API secret and also verified that it worked. |
Beta Was this translation helpful? Give feedback.
-
Spiffy. Thanx for keeping the beat while we ran off on a tangent. Your
changes to handle this change look right to me. Thanx!
…On Sun, Dec 24, 2023 at 11:50 PM Marshall G Gates ***@***.***> wrote:
I updated the pull request it the necessary tests for the stock ticker API
secret and also verified that it worked.
—
Reply to this email directly, view it on GitHub
<#572 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD32FY2WFHDNPGQBW7HTYLEH2RAVCNFSM6AAAAABBALEBISVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM3TSNBRHA4TM>
.
You are receiving this because you were mentioned.Message ID:
<PlummersSoftwareLLC/NightDriverStrip/repo-discussions/572/comments/7941896
@github.com>
|
Beta Was this translation helpful? Give feedback.
-
Was just tinkering with mr gates code tonight with the stock ticker. Have a question, if this feature is added to the nightdriverstrip main branch soon. Can you then also upload the mesmerizer if you don't fill in a stock api? Because tried to upload the branch of mr gates on my mesmerizer, but get an error message because I didn't fill in the stock api. I'm not into stocks. So am curious if this feature is added to the now existing branch, will you be able to upload it without stock api.
Harrie
Beta Was this translation helpful? Give feedback.
All reactions