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

Patch 1 #36

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Patch 1 #36

wants to merge 2 commits into from

Conversation

dogmatix12
Copy link

alter 'getter' functions: return uint8_t istead of int

functions return smaller data type (byte istead of integer) as defined in 'tmElements_t'
use byte istead of int (smaller program, faster execution)
@michaelmargolis
Copy link
Collaborator

The types were chosen to be consistent with other Arduino APIs at the request of the Arduino team . My initial version of this library did use eight bit types for the time properties, but this was changed to the current form when the Arduino team reviewed this library and suggested using ints. At this point I do not see much to be gained by changing the types of properties exposed by the API.

@dogmatix12
Copy link
Author

ok, I need to save space in PROGMEM, so i made this change. (using 16-bit values does not make sense to me)

@michaelmargolis
Copy link
Collaborator

how many bytes do you save if this is the only change? - just curious

@dogmatix12
Copy link
Author

  • in my code, it is about 100 Bytes
  • this (senseless) code is smaller by 26B:
typedef uint8_t UNIT_TYPE[3];
UNIT_TYPE t;

void loop() {
  t[0] = hour();   t[1] = minute();  t[2] = second();

  if (t[0] > 10 && t[0] < 12 && t[1] == 0) {
   delay(100);
  }
  delay(1000);
}

@PaulStoffregen
Copy link
Owner

David Mellis made those comments years ago. It's pretty clear his original intention to put this into the core library has long since been abandoned. If they ever consider merging again, I'm pretty sure this won't matter one way or the other.

However, this change will (probably) be less efficient on 32 bit chips, which are probably the future of all microcontrollers. On those platforms, using smaller than 32 bits or "int" often causes extra instructions to clear or sign extend the upper bits, since every register is always 32 bits, even if holding 8 or 16 bit data.

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.

3 participants