SIX FRIKING BYTES! making a code SIX BYTES smaller

forgive my enthusiasm but after working 26 hours on something, and getting stuck where you least expect.. i cant hold back.

So basically this code

const int relayPin =  3;
const int voltageRead = A2;
unsigned long currentMillis;
unsigned long previousMillis = 0;
void setup() {
  pinMode(relayPin, OUTPUT);
  pinMode(voltageRead, INPUT);
  pinMode(2, OUTPUT);
}

void loop() {
  tone(1, 65000);
  if (analogRead(voltageRead) >= 193)
  {
    digitalWrite(relayPin, HIGH);
  }
  else {
    digitalWrite(relayPin, LOW);
  }

  currentMillis = millis();
  if (currentMillis - previousMillis >= 15000)
  {
    previousMillis = currentMillis;
    digitalWrite(2, HIGH);
    currentMillis = millis();
    if (currentMillis - previousMillis >= 3000)
    {
      digitalWrite(2, LOW);
      previousMillis = currentMillis;
    }

  }
}

has to fit inside 1024 bytes, but it compiles at 1030 bytes. I have no idea what to change here, I'm not too familliar with port manipulation to use it, and im using microcore by spencekonde ( your awesome). The microcore github listed that i can disable few un-necessary functions to save space, but im not idiotic enough to do something on a hunch, that maybe i dont need the function...

I have tested the code on a Nano, so the code works just fine, so ill spare you the details and comments. yes, it isnt quite readable, im sorry, really am, please just bare with me for this while.

  • i don't think you need to call millis() again to set currentMillis()
  • instead of conditionally calling digitalwrite() why not conditionally set a variable to the state you want

This may work for you: https://forum.arduino.cc/index.php?topic=694728.0

It is easy to use direct port manipulation, and it would save significant space to eliminate the digitalRead/Write section from the core.

These two lines set PORTB, bit 0 to OUTPUT, then to HIGH, and compile into two bytes of code, total.

DDRB |= 1;
PORTB |= 1;

Not using currentMillis saved me 10 bytes.

Using the digitalWriteFast.h library saved me 18 bytes

...R

aarg:
Any number greater than 15000 is also greater than 3000. So the second test of currentMillis will always return 'true'. So, we now have

after waiting 15 sec, setting the output HIGH and resetting previousMillis, he wants to wait 3 more seconds and set the output LOW.

kaseftamjid:
I have tested the code on a Nano, so the code works just fine, so ill spare you the details and comments. yes, it isnt quite readable, im sorry, really am, please just bare with me for this while.

Well that is pretty smug. I find it hard to believe that. Look at the pin 2 (the obfuscated pin) logic. Every 15 seconds, you set it high. Before exiting that block, you refresh the timing variables such that both 'currentMillis' and 'previousMillis' are approximately equal to millis(). Then you test to see if the difference is greater than 3000 (3 seconds). It seems to me, that can never happen. So pin 2 can never be set LOW.

gcjr:
after waiting 15 sec, setting the output HIGH and resetting previousMillis, he wants to wait 3 more seconds and set the output LOW.

You can't perform any waiting without exiting the block, because of the way it's nested. The test for 3 seconds elapsed is unreachable until another 15 seconds has elapsed.

aarg:
You can't perform any waiting without exiting the block, because of the way it's nested. The test for 3 seconds elapsed is unreachable until another 15 seconds has elapsed.

I .. didnt notice that, thanks.

After a bit of intense staredown, this is the code now.

const int relayPin =  3;
const int voltageRead = A2;
unsigned long currentMillis;
unsigned long previousMillis = 0;
bool state;
void setup() {
  pinMode(relayPin, OUTPUT);
  pinMode(voltageRead, INPUT);
  pinMode(2, OUTPUT);
}

void loop() {
  tone(1, 65000);
  if (analogRead(voltageRead) >= 193)
  {
    digitalWrite(relayPin, HIGH);
  }
  else {
    digitalWrite(relayPin, LOW);
  }

  currentMillis = millis();
  if (currentMillis - previousMillis >= 15000)
  {
    previousMillis = currentMillis;
    digitalWrite(2, HIGH);
    currentMillis = millis();
    state = true;
  }
  if ((currentMillis - previousMillis >= 3000) && state == true);
  {
    digitalWrite(2, LOW);
    previousMillis = currentMillis;
    state = false;
  }


}

gcjr:

  • i don't think you need to call millis() again to set currentMillis()
  • instead of conditionally calling digitalwrite() why not conditionally set a variable to the state you want

im really a beginner to programming. Could you please elaborate?

dougp:
This may work for you: https://forum.arduino.cc/index.php?topic=694728.0

Unfortunately, this library doesnt support millis.
edit: digitalwritefast also doesnt work.. afaik, its a timer issue

gcjr:
after waiting 15 sec, setting the output HIGH and resetting previousMillis, he wants to wait 3 more seconds and set the output LOW.

yes thats exactly what im trying to do.

i ran a few simulations and the code doesnt work..

kaseftamjid:
Unfortunately, this library doesnt support millis.
edit: digitalwritefast also doesnt work.. afaik, its a timer issue

What library does not support millis() ?

What microprocessor are you trying to compile for?

...R

Robin2:
What library does not support millis() ?

What microprocessor are you trying to compile for?

...R

dougp:
This may work for you: https://forum.arduino.cc/index.php?topic=694728.0

this library that i was suggested.

Im trying to compile for an attiny13A

aarg:
Well that is pretty smug. I find it hard to believe that. Look at the pin 2 (the obfuscated pin) logic. Every 15 seconds, you set it high. Before exiting that block, you refresh the timing variables such that both 'currentMillis' and 'previousMillis' are approximately equal to millis(). Then you test to see if the difference is greater than 3000 (3 seconds). It seems to me, that can never happen. So pin 2 can never be set LOW.

i really have no excuses... the led turned on and i didnt wait 15 sec to see what would happen.

im using microcore by spencekonde ( your awesome).

Um. Spence is awesome, but "microcore" is by Hans (MCUDude.)
assuming you mean GitHub - MCUdude/MicroCore: An light-weight Arduino hardware package for ATtiny13 ?
(I don't think that Spence has an attiny13 core.)

Do you have a recent update of this board package? When I compiled the program from your first post here, it was only 508 bytes... (There are some relatively recent updates, like NerdRalph's picoUART code (not that that should affect your sketch.))

westfw:
Um. Spence is awesome, but "microcore" is by Hans (MCUDude.)
assuming you mean GitHub - MCUdude/MicroCore: An light-weight Arduino hardware package for ATtiny13 ?
(I don't think that Spence has an attiny13 core.)

Do you have a recent update of this board package? When I compiled the program from your first post here, it was only 508 bytes... (There are some relatively recent updates, like NerdRalph's picoUART code (not that that should affect your sketch.))

oh, i messed up between microcore and attinycore. thanks for pointing it out.

There was this avr update in library maneger, and it seems to drop my code to 536 bytes. doesnt make sense as im not using that...

After being overtly smug on the code i wrote, I literally have no trust on my coding abilities. could someone please point me why this code isnt working? i mean on the logic end?

const int relayPin =  3;
const int voltageRead = A2;
unsigned long currentMillis;
unsigned long previousMillis = 0;
bool state = false;

void setup() {
  pinMode(relayPin, OUTPUT);
  pinMode(voltageRead, INPUT);
  pinMode(2, OUTPUT);
}

void loop() {
  tone(1, 65000);
  if (analogRead(voltageRead) >= 193)
  {
    digitalWrite(relayPin, HIGH);
  }
  else 
  {
    digitalWrite(relayPin, LOW);
  }

  currentMillis = millis();
  if (currentMillis - previousMillis >= 15000)
  {
    previousMillis = currentMillis;
    digitalWrite(2, HIGH);
    state = true;
  }
  if ((currentMillis - previousMillis >= 3000) && state == true);
  {
    digitalWrite(2, LOW);
    previousMillis = currentMillis;
    state = false;
  }


}

relayPin does not go High and Low? What is connected to A2?

No semicolons at the end of an if.