Skip to content

XBeeAddress64 improvements #6

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

Merged
merged 3 commits into from
Aug 4, 2015

Conversation

matthijskooijman
Copy link
Contributor

This implements the suggestions from #3, and a bit more. The commit logs have more info, but roughly:

  • This allows converting XBeeAddress64 to and from uint64_t transparently
  • This optimizes the XBeeAddress64 class

All existing API is still supported, working, and should not any slower or bigger. These changes have been compile-tested against all examples, as well as runtime-tested with my own programs.

The new APIs using uint64_t are significantly bigger and slower than the existing APIs, because the compiler doesn't properly optimize uint64_t operations, so none of the other code and examples have been updated to use the new APIs yet.

I have a few more improvements planned (#5). If this PR is merged already, I'll open a new one, otherwise I'll append those here.

Closes #3.

This adds:
 - A constructor that takes a single uint64_t variable. This allows
   implicitly converting integers to XBeeAddress64 values, so these will
   work:

        XBeeAddress64 dest = 0x0;
        request.setAddress64(0x0013A21234567890);

   (note that the latter also requires some const-correctness fixes in
   methods taking XBeeAddress64 arguments).
 - A get() and set() method for getting and setting the address as a
   uint64_t.
 - A operator uint64_t() which allows the XBeeAddress64 to be
   implicitely converted to a uint64_t. E.g.:

        uint64_t address = resp.getRemoteAddress64();

   But also comparison:

        resp.getRemoteAddress64() == XBeeAddress64(0, 0)
        resp.getRemoteAddress64() == 0x0013A21234567890);

   Note that both comparisons above work by converting to uint64_t and
   comparing those.

   Because this removes the need for separate comparison operators, the
   ones that were there (but still commented out) were removed.

Note that avr-gcc doesn't currently optimize uint64_t values very well,
so the storage is still left as two separate uint32_t values. If you use
the msb/lsb accessors, code should be unchanged compared to before, but
if you use the uint64_t accessors or compare addresses, code will be
less efficient. This will hopefully be fixed in a future compiler
version, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66511.
Since this is a utility class that should really be a very thin wrapper
around the actual data, the compiler should be able to inline all
operations, instead of having to do a full function call just to load or
store a few bytes.

This removes the explicit super-constructor calls and rewrites the
constructors to use initializer lists instead of assigning values in the
body (which is more consise, and allows for constexpr next)

This saves up to a 30 bytes of program spaces on the supplied
examples, probably more for more complex programs.
This allows the compiler to better optimize (global) XBeeAdress64
instances.

With the examples sketches, this saves between 44 and 114 bytes of
program space, compiling for the Uno. Even sketches that do not directly
use any addresses shrink, because the initialization of
RemoteAtCommandRequest::broadcastAddress64 can be simplified.

This feature requires C++11 to be enabled. This isn't currently the case
(with Arduino 1.6.4), but is expected to be enabled in a future release.
A preprocessor check is used to ensure that the code compiles with and
without C++11.
andrewrapp added a commit that referenced this pull request Aug 4, 2015
@andrewrapp andrewrapp merged commit ce5f8cf into andrewrapp:master Aug 4, 2015
@matthijskooijman matthijskooijman deleted the improvements branch August 6, 2015 19:17
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.

2 participants