Skip to content

svcd unsubscribe#38

Closed
mccormickt12 wants to merge 10 commits intoUCB-IoET:masterfrom
mccormickt12:master
Closed

svcd unsubscribe#38
mccormickt12 wants to merge 10 commits intoUCB-IoET:masterfrom
mccormickt12:master

Conversation

@mccormickt12
Copy link

not really sure how to do this... i think i submitted everything ive done for a pull request...

@mccormickt12
Copy link
Author

sorry about the terrible git use. The svcd file is at the bottom of all the changed files

@kvsikand
Copy link
Contributor

kvsikand commented Mar 1, 2015

Looks good; but add a return statement! Looks like the function should be returning an int. I think you should be returning 0 to indicate that you want 0 return values from the function.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to register your function at the beginning of this file under SVCD_SYMBOLS. There is a template for the init function already so you just need to follow it.
PB4J

@leonardt
Copy link

leonardt commented Mar 1, 2015

You can try the instructions I detailed here to get a fresh pull request with only the svcd.c changes without having to do any complex git commands

@samkumar
Copy link
Contributor

samkumar commented Mar 1, 2015

@Michaelhobo The implementation of Lua that runs on Storm doesn't support floating point numbers at all. Everything is an integer. So I doubt that lua_Number would be a double in this case.

-men in #000000

@jwqchen
Copy link

jwqchen commented Mar 1, 2015

Some of the chunks could be abstracted into functions. For example:
lua_pushlightfunction(L, arr_set);
lua_pushstring(L, "msg");
lua_pushnumber(L, 1);
lua_pushnumber(L, 0);
lua_call(L, 3, 0);
Should be available as a single call:
send_msg(arr_set, "msg", 1, 0);
and:
lua_pushlightfunction(L, arr_set_as);
lua_pushstring(L, "msg");
lua_pushnumber(L, ARR_TYPE_UINT16);
lua_pushnumber(L, 1);
lua_pushnumber(L, svcid);
lua_call(L, 4, 0);
as:
send_msg(arr_set_as, "msg", ARR_TYPE_UINT16,1, svcid);

-- BIRDPEOPLE

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why we should push a string "msg" to the stack. Although msg is the name of the array variable, it is never needed for it to be represented this way. Moreover, if you want to push the same string "msg" twice, it would be more memory efficient to save it as a constant array and push it twice.

Untitled

@immesys
Copy link
Contributor

immesys commented Mar 1, 2015

This pull request is not at all in the spirit of feature branches and pull requests in general. When you are submitting your final pull request (due 11:59 tonight) please make sure to only submit changes related to the SVCD work, not every other file you have changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a merge conflict. You should never commit code with an unresolved merge conflict

@immesys
Copy link
Contributor

immesys commented Mar 1, 2015

You are using storm array functions, which is acceptable as that is what is found in the lua, but it is not the most efficient. The reason we are using those functions in lua is because we lack the ability to work with raw bytes in a string, setting them as numbers. In C you are free to create the message just using a uint8_t array and setting the bytes in the array directly. This is much faster than using the storm array functions.

@immesys
Copy link
Contributor

immesys commented Mar 2, 2015

This has been superceded by #42

@immesys immesys closed this Mar 2, 2015
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.

10 participants