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

Drive by comments #2

Open
robertlipe opened this issue Feb 10, 2024 · 6 comments
Open

Drive by comments #2

robertlipe opened this issue Feb 10, 2024 · 6 comments

Comments

@robertlipe
Copy link

Hey, Sean!

Looks promising! For someone that claims to be just starting, you're getting a lot of things right, but I hope you're open to some help awesomeizing things here and there. It's late and I'm just freehanding, so anything hta tlooks like code but doesn't actually compile is just me being a dumb. It should be inspirational enough that one of us can go to cppreference.com or github search and make it make sense.

void snmpLoop(int Array[], int arrayCount, int arrayIndex);

C style {array[], int array_size} pairs are a drag. Carrying around to things that describe a pile of stuff sucks. Put them in a container. There are a bunch, but I'll just recommend two:
std::vector array; You can append things easily. You can insert in the middle, but that's slow. array.size() always contains the size. YOu called array.push_back(new_packet) ? array.size() just grew. Now you can walk the array C style, but now you have icky loop iterators.
for (auto& port : hub) {
port.SetLed(on);
}

So std::vectors are cool, but you already have C-style arrays. They recently (C++20?) added std::array that makes plain old staticly sized arrays act like normal C++ containers. Once you see examples, I think the proverbial LED will go off for you:
https://www.geeksforgeeks.org/stdarray-in-cpp/
https://www.geeksforgeeks.org/vector-in-cpp-stl (I don't like all their examples. I prefer "for (auto& thing : things){thing.DoStuff();}" to "for (int i = 0; i < things.size(); i++){things[i].DoStuff();}"

Rule of thumb: compile-time fixed size? (number of LEDs) std::array. If you need a container that can grow? (receive packet buffer) std::vector. You can earn hashes later.

snmp's createArrays should really fall out nicely with std::array

The (in|out){1,2,3,4}Total stuff should probably be:
const int kThingyCount = 4;
struct ThingyData {
unsigned long onTotal;
unsigned long outTotal;
mumble lastInOctets;
something responseInOctets;
};
std::array Thingies[kThingyCount];

Now
void handleAllOutputs(std::array& ThingiesInput) {
for (int i = 0 ; kThingyCount; i++) {
Thingies[i].lastInOctets += ThingiesInput[i]; (or something - I don't really grasp th relationships here. But you clearly have four of something that have a relationship to some number of something elses. The data structures should represent that. I think that
setTotals() {
for (int i = 0 ; kThingyCount; i++) {
thing[i].totalsIn += Totals[i].in;
}
}
}

// My examples may be making this worse because I'm not understanding the (lack of) data structures now used. I think you have all the right pieces identified, you just need to put them in structs and arrays so you can operate on multiples.

Math!

This could be very cool, but it's also low-value. You can design key->value pairs and place them in a map. Then use std::lower_bounds to find the lowest KV pair that matches the search value. Return the found key's value and you're done. The whole file reduces to three tables and a few lines of code. It's something approximately like:

map<int key, int value>table;
const table = {
{1000,1}
{10000,2}
{35000,3}
{100000,5}
};

// Now these are in a container you could iterate for debugging:
for (auto x : table) {
cout << x.first << "->" <<
x.second <<endl;
}

// If these tables were large, we could call std::lower_bound, but it's probably clearer to just code it:

int rv;
int find_key(value) {
for (auto x : table) {
if x.value >= value
return rv;
rv = x.value; // save the previous one
}
return 17;
// It's the kind of thing to do with unit tests, for sure.

main.cpp
make constants const;
const int kPollInterval = POLL_DELAY;
Pull all these
unsigned long outPulseInterval3 = 0;
into a struct if they're related, then, if there are always four of them, put them in a std::array of those[4]; (or kNumberThingies or whatever)

#include <snmpgrab.h> use "" if it's local project and <> if system

lablights
// figure out Strip[1-4]
That comets[] loop gets easier/clearer
for (auto& comet : comets) {
if comet >= 0 && comet <= TotalLeds
leds[comet] += forwardColors... oof. Maybe we need the index.

if (constexpr (PROJECT_NAME == "Lablights")) {
That lets the code get omitted at compile-time if it's not true

Think about nerd math to make the reverse and forward cases the same.

fadeAll() - clever, but I think that fadeLightBy https://fastled.io/docs/group___color_fades.html does this.

forward,reverseEvent() - fid a way to eat that number of strips into 1 body. 8 bodies is too many.

I know it seems odd, but I'm excited to see this. I knew when we talked months ago that you had a strong vision of how you wanted this to work and the determination to brute force it into reality. You have the appetite to make this awesome and I have a bit of mentoring time on my hinds. (I can also be grating as hell, sticking my nose in where it wasn't asked for. :-) )

I'm sure you'll take some of this and run with it. Some if it just doesn't matter (like maybe reducing that math stuff to tables). Some of it won't make any sense at all because I'm sleep-deprived and talking out of turn.

I don't know anything about SNMP, but if you have some kind of ability to dummy up/mock things for testing and I can test individual strips/efffects, I'm willing to help with some of the bit-slinging just because I think this is a cool project.

TTYS!

@SeanMcKeen
Copy link
Owner

Hi, I appreciate your enthusiasm and mentoring (god knows I need it). Here's the thing, I understand none of this whole std::array and std::vector thing. Every time I saw that type of thing in code examples my brain just exploded. However, I figured out vectors (BARELY), just enough to replace one thing with them. And i do really love being able to just use arr.Size(); as well as now just looping with for (const int o : Array) {}

Here's my big issue, I can't figure out why, but the code is printing out port 1, 2, 3, and 4. and then where its supposed to be 5, 6, 7, and 8. It just keeps printing out 3, 4, 3, 4. NO CLUE why, I even tried removing variables and just putting snmpLoop({5, 6}, 3); and even that doesn't work.

I honestly have no clue how c++ works with some of these more complicated things, and thats why I stuck with the simplest of the simple even if it was CLUNKY as hell.

@robertlipe
Copy link
Author

robertlipe commented Feb 13, 2024 via email

@SeanMcKeen
Copy link
Owner

SeanMcKeen commented Feb 28, 2024

After weeks of brain damage from attempting to rework and fix the code the best I could. I still can't seem to find the error. Even without SNMP even being used, the lights crash and don't reset the esp. No clue what this could be, I was assuming memory write errors but now I have no clue.

I uploaded it to the testing branch once again hoping someone else can see what I'm doing wrong. I'm at the point where I can't seem to find a solution no matter how much research and head banging I do.

About a virtual way to test this... I have no idea how to do any of that.

@robertlipe
Copy link
Author

robertlipe commented Feb 29, 2024 via email

@SeanMcKeen
Copy link
Owner

I believe that MOSTLY everything that needs to be configured should be in either globals.h or secrets.h. Other than that since you got it to work with the lilygo env, I THINK it should be fine to work just like that.

On to what it's not doing that I need it to do is just the crashing. I seem to be able to get it to poll the device and work fine for around 10 seconds, and then all of a sudden it just dies. It stops printing, the leds stop updating, dead.

USUALLY, when this happens I've been getting a guru meditation error on core 1, looking it up online said it was a memory issue. I checked the free heap size and it seems perfectly fine so I keep thinking there's a memory leak.

However, I tried completely removing the necessity to have snmp data polling in there and just give my program a fixed number to work with (500,000). This STILL causes the program to crash and I have no clue why or where as I have it using dynamic memory so the memory addresses tell me nothing.

@robertlipe
Copy link
Author

robertlipe commented Mar 4, 2024 via email

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

No branches or pull requests

2 participants