# Tag Info

26

Toggling? Or setting? Our toggleLights function is oddly named and doesn't do what I'd expect. (Specifically, a toggle turns off things on and on things off.) In C, and especially in embedded C, we shouldn't be afraid to use unsigned integers as bit arrays. So, perhaps we want a setLights function that looks something like this: void setLights(uint8_t ...

17

how to optimize this code regarding memory consumption and performance. (?) Consider redefining the angle measurement. It appears int phase is 0.36 degrees. A natural choice would be 1024 phase to 1 revolution. Now the problem becomes one using a binary angle measure. Not only does this simplify small things like scaling, it opens up coding choices as ...

17

Do not call main recursively. You are setting yourself up for stack overflow. Consider instead def main(): while True: try: your_logic_here except Exception as e: your_logging_here Testing for counter == 4 is better done in a loop: for _ in range(4): handle_acceleration handle_the_rest An ...

14

First of all, yes, CortexM0 lacks any way to do 32x32=64 multiplication in hardware. CortexM3 and CortexM4 have the umull instruction, which lets you do 32x32=64 really easily. And yes, since you're writing in C, one possible implementation would be uint64_t mul32x32(uint32_t r0, uint32_t r1) { return r0*(uint64_t)r1; } but I assume you've already tried ...

13

Good that OP is using 4 simplifications: year 2000-2099, no DST, no leap second, no timezone. So OP knows of code limitations concerning these. Various elements of this function break without those givens. Make static unsigned short days a const. Use a long for your epoch as in: void epoch_to_date_time(date_time_t* date_time,unsigned long epoch) as ...

13

Didn't work for me First off, I ran your program but it didn't ever find any word. I suspect the problem was in the function max: if (e && ((int) e->data > max_size)) I didn't see anywhere in the program where data was ever incremented, except if you had a duplicate word in the dictionary. My dictionary had no duplicate words. Perhaps ...

12

...it [the program] is to be used in embedded devices with possibly very low available RAM In that case, we should be getting rid of everything that isn't of absolute necessity, and adding some things that are. Overall: If one provides no memory modifier (such as __flash) then many embedded systems compilers will copy the data into RAM (even sometimes ...

12

Without more of the code, it's not really going to be possible to address memory management or bottlenecks. So instead, I'll look instead at readability and portability. Use the appropriate #includes This program fragment requires two headers, which should be included but are not: #include <stdlib.h> // for getenv(), srand(), fprintf(), etc. #...

12

Have you already executed the code to see how it performs and if the battery will last? There is that famous Donald Knuth quote saying premature optimization is the root of all evil (or at least most of it) in programming. I never had to think about the energy consumption of a program, so I cannot tell you about the power efficieny. But as vnp already did, ...

11

A few notes: Your included libraries and definitions at the beginning of your code is not very organized. #include <avr/io.h> //#undef __FLASH #ifndef __FLASH #include <avr/pgmspace.h> #define FLASH(x) const x PROGMEM #define FLASH_P(x) const x * const PROGMEM #define FLASH_PR(x, y) (x *)pgm_read_word(&(y)) #else #define FLASH(x) const ...

11

I see a number of things which could help you improve your code. Minimize register usage With assembly language programming, and in particular in embedded systems work, minimizing the use of resources is often vital. One of the most precious resources is the processor's registers. In this case there are only 32 of them, so minimizing their use is often ...

11

Illegal void pointer arithmetic This code is illegal in C: void *array; ... (array + offset) You are not allowed to do pointer arithmetic on a void * pointer. You should use char * instead of void *. Simplifications I would suggest defining a generic structure like this: typedef struct EnumName { int value; const char *name; } EnumName;...

10

Magic numbers You have some magic numbers in your code. Create constants for them to make the code more readable and maintainable. Using an unnamed enum is a trick for creating constant int values. Using const qualified variables work in this case, but don't for things like in switch statements and for the size of arrays. See this answer for more info on ...

10

Your current delay code is busy-waiting, this means that in order to delay for the amount of time required you are just wasting however many CPU cycles you need in order for that amount of time to elapse. If I remember correctly _delay_ms is the library function that does something similar to what you already have. It works by calculating the number of no-op'...

10

Pointer issues: You should avoid void* where it isn't necessary. In this case, there is no reason why you should use it, use a char* instead. Pointer arithmetic on void* in not even valid C, this is a non-standard GCC extension. You should use const-correctness for the pointers where the contents aren't modified. (int) * ((int *)something) The cast to int ...

9

<stdbool.h> Unless you need compatibility with C89 for some reason, I would use bool for the return type of the function and true and false as the possible values. It helps readability. sscanf() This one depends on your performance needs. A solution using sscanf() will be much easier to understand with a simple look and also shorter in code, but ...

8

Two-dimesional array Your first function can be aggressively reduced by replacing cI and friends by a 2D array: float *Receiver::parse_pid_substr(char* buffer) { static float pids[8]; memset(pids, 0, 8*sizeof(float) ); char rgcPIDS[8][32]; size_t i = 0, c = 0, p = 0; for(; i < strlen(buffer); i++) { if(buffer[i] == '\0') { break; ...

8

Opening and closing files takes resources: with open('babar.txt', 'a') as f: f.write('a'*10000) takes 300 micro-seconds while: for _ in range(10000): with open('babar.txt', 'a') as f: f.write('a') takes 648000 micro-seconds So to answer your question Would it be beneficial to write 10 rows at once instead of writing one row at a time?. The answer, ...

7

Standard C# naming convention for methods is PascalCase. Following standard naming conventions makes the code look more familiar to other C# developers (which might be important as you plan to open source it) You are potentially wasting some cycles here by calling getMaxBulbs() again even though you just did it and have the result stored in maxBulbs: Int32[,...

7

This really is picky, picky, picky, and I recognize that. However, source code is written for human beings, not computers, so your source code should be crystal clear, and as readable as possible. Keep this in mind. There's nothing harmful in your header, led.h, but you didn't wrap it so that it's included only once. You can see this practice in all sorts ...

7

here are the programming principles you ought to be applying: Don't repeat yourself Note that the two statements (statement-true & statement-false) are just about the same as each other. If there's a mistake in one and you fix it, you might not fix it correctly in the other. So you really should have just one copy of the formula. This is even more ...

7

I'm interested in how to optimize this code regarding memory consumption and performance. Well, which is it? Are you optimizing for speed, ram, or executable size? You say this is for an embedded project, so it’s likely you’ll need to pick one. Once I needed to calculate a sine wave for a motion controller. The timing was critical, but we had plenty of ...

6

I would focus on eliminating the inner switch statements. You could try to eliminate the outer switch as well, but it's probably not worthwhile, especially for a resource-constrained environment. byte[][] desKeys = new byte[][] { null, DESKey1, DESKey2, DESKey3, DESKey4 }; try { switch (buf[ISO7816.OFFSET_P1]) { case 0x01: ...

6

It's a small point in comparison to the other answers, but worth noting for a beginner. This is the LED header file. #include <avr/io.h> #define LEDPORT PORTE_OUT #define LEDPORT_DIR PORTE_DIR void init(void); void toggleLights(int ledPosition); Anything in the header is available to outside code. There's no reason for anything to have access to ...

6

A few thoughts about your code: 1. Use of reinterpret_cast // Create a simple function that calls the functor this->func_ = [](void *user, TArgs ... args) -> void { TFunctor *functorPtr = reinterpret_cast<TFunctor*>(user); (*functorPtr)(args...); }; This always raises a red flag for me. I understand that the user ...

6

Improve the wrap-around logic There's obvious inefficiency in: head %= bufferSize; % uses division, but because we're incrementing, we know that head / bufferSize is at most 1. Instead, we can: if (++head >= bufferSize) head -= bufferSize; On architectures such as ARM, there's no branch here, as any decent compiler will just use a condition flag ...

5

5

At the top, you are declaring eight 32-character arrays, which later you use a switch with to decide which array to use. Instead, you should declare an array of arrays, the Morwenn did it, or you can use just one 256-character array. You can then just add a multiple of the p to the index in the first big switch block, reducing it to only one case: cArrays[p&...

5

Since your script has no content to return, a status code of 204 No Content would be more desirable than 200 Success. For that, you should echo "Status: 204 No Content" (RFC 3875 Sec 6.3.3). Also consider returning using status code 405 Method Not Allowed for anything other than a POST request. $TMPOUT is a misnomer. The file is not temporary at all —$...

5

Here are some things that may help you improve your program. Use all required #includes The program uses fopen but doesn't #include <stdio.h>. It should. Use const where practical Whenever you pass a pointer, ask yourself whether the called function should be allowed to modify the contents of the pointed-to memory. If not, then that parameter ...

Only top voted, non community-wiki answers of a minimum length are eligible