flipper-io / flipper

Flipper is a development platform that can be controlled from any programming language.
https://www.flipper.io/
Apache License 2.0
70 stars 15 forks source link

Implement variable modification machinery. #80

Open georgemorgan opened 7 years ago

georgemorgan commented 7 years ago

High level implementation should have a way to modify global variables defined in a module. The machinery for the modification of these variables should be incorporated directly into the message runtime system and be handled as a discrete message runtime subclass.

kjcolley7 commented 6 years ago

With the new symbol-aware loader that's in progress, this would be fairly easy to accomplish. There could be a dlsym() style message where you ask to load a symbol from module "led" named "led_rgb" (or maybe just ask for "led.rgb") and get back its loaded address.

georgemorgan commented 6 years ago

I was thinking we we could use a pragma similar to LF_FUNC called LF_VAR that just autogenerates setters and getters for the variable and exposes them as module functions that can be called from by the message runtime.

kjcolley7 commented 6 years ago

So something like this?

#define LF_VAR(varName) \
LF_FUNC void set_##varName(__typeof__(varName) newValue) { \
    varName = newValue; \
} \
LF_FUNC __typeof__(varName) get_##varName(void) { \
    return varName; \
} \
LF_FUNC __typeof__(varName) get_##varName(void)

// Use like:
int someGlobal;
LF_VAR(someGlobal);

Keep in mind that this will create an extra two functions and symbols per global variable you mark with LF_VAR. An alternative implementation would be something like this:

#define LF_VAR __attribute__((__visibility__("default")))

// Use like
LF_VAR int someGlobal;

In that case, LF_FUNC should also include __attribute__((__visibility__("default"))), and the CFLAGS used to build modules should contain -fvisibility=hidden. Then, the loader could handle messages like GET_VARIABLE and SET_VARIABLE that take a qualified name like uart0.idx.

georgemorgan commented 6 years ago

Either way, we will have to export two functions as the setter and getter for the variable. Whether those live in the module binary is up for discussion, but the bindings will still need those symbols exposed to wrap the message runtime calls to the loader.

kjcolley7 commented 6 years ago

From the binding, you could send the GET_VARIABLE and SET_VARIABLE messages instead of calling separate functions.

georgemorgan commented 6 years ago

We still need to export the symbols that send those messages.

void my_variable_set(type_t value) { lf_set_variable(&_module, "varname", value); }

and

type_t my_variable_get(void) { return lf_get_variable(&_module, "varname"); }

kjcolley7 commented 6 years ago

But that's just two functions total. The alternative is two functions per variable.

georgemorgan commented 6 years ago

See updated comment for clarity. You will still need two functions per variable.

kjcolley7 commented 6 years ago

That's on the host's side though as opposed to the Flipper, right? Who cares about the binary size of the host's bindings? The host can send the (GET/SET)_VARIABLE messages directly to the flipper, so the flipper itself wouldn't need the extra functions per variable

georgemorgan commented 6 years ago

Yes, we are on the same page now. Although, if you do it that way you would have to do a string lookup on every setter and getter call unless you give variables indices as well.

kjcolley7 commented 6 years ago

That would work if we pull the entire symbol table from the module lazily. So if it's loaded from the host or the host wants to interact with the module, it'll pull the entire symbol table and reconstruct it in libflipper's memory with the symbol indices rather than values.

georgemorgan commented 6 years ago

I think that doing things that way is objectively better, but I'm trying to figure out whether it is entirely necessary for the 99% use case where the module and bindings are in sync. Calls are expensive, especially over a network, and syncing the symbol table every time could be extremely bad for the performance in certain situations.

nicholastmosher commented 6 years ago

I would strongly ask against having two host-side getter/setter functions per variable, because that significantly increases the amount of "ffi"ing we need to do in the bindings. I think that preferably, there would be a unified "variable access" API exposed on libflipper. Similarly to how the arguments lf_invoke describe the location, index, and expected return type of any fmr function interacted with. The variable interface could be similar, perhaps some sort of lf_value_get and lf_value_set that accepts a module location, name, and type information.

The biggest advantage to all of this is that there is a fixed amount of FFI that high level bindings need to do, which then scales with no marginal cost regardless of how many instances of use it has. It also makes the binding story (particularly for rust but likely other languages) much safer because we have a small, airtight, battle-proven foreign interface that works for everything, rather than having to recreate new ffi bridges for each new use.

georgemorgan commented 6 years ago

@nicholastmosher, if what you mean by "ffi"ing is the number of bindings we will need to generate, it will be the same whether we have the setter and getter on the device or whether we implement this "variable access" API. How we implement it device side has no bearing on how the bindings will be generated, or the number of message runtime calls needed to get or set a variable value.

I think that what I'm failing to make clear here is that you will still need a setter and getter for the variable exposed on the binding side. We can eliminate the need to have the actual setter and getter functions per variable on the device by replacing them with the lf_value_get and lf_value_set functions in the loader / libflipper.

The bindings will either look like this.


// on the device:
// int foo;

void foo_set(int value) {
    lf_invoke(&module, _foo_set, fmr_void, fmr_args(fmr_int(value)));
} 

int foo_get(void) {
    return lf_invole(&module, _foo_get, fmr_int, NULL);
}

OR


// on the device:
// int foo;

void foo_set(int value) {
    lf_variable_set(&module, "foo", fmr_int, value);
} 

int foo_get(void) {
    return lf_variable_get(&module, "foo", fmr_int);
}

Whether we call a function on the device that is the setter and getter, OR we use some built in API, the number of functions we have to create and the number of calls needed to perform the operation stays the same.

That said, the latter is better because it reduces binary size on the device. However, due to the need to sync the symbol table and resolve symbol names to indices at bind time, it may not be the best solution.

nicholastmosher commented 6 years ago

Ok, I think we're on the same page. Essentially what I want to avoid is having this:

// foo.rs
extern {
    fn foo_set(libc::int value);
    fn foo_get() -> libc::int;
}

struct Foo;
impl Foo {
    fn set(u32 value) {
        unsafe { foo_set(value) }
    }
    fn get() -> u32 {
        unsafe { foo_get() }
    }
}

As long as we have something like lf_invoke or lf_value_get/lf_value_set, we should be able to implement it like this:

// bar.rs
struct Bar;
impl Bar {
    fn set(u32 value) {
        lf_value_set(/* module, name, type, value */)
    }
    fn get() -> u32 {
        lf_value_get(/* module, name, type */)
    }
}

The key difference being that we do not introduce a growing number of extern functions as we introduce more variable accessors.