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

Added spark internal rgb support #49

Merged
merged 1 commit into from
Apr 1, 2015
Merged

Conversation

canda
Copy link
Contributor

@canda canda commented Mar 29, 2015

According to changes on this pull request
voodootikigod/voodoospark#40
I added the function for controlling spark rgb internal led.
This only works on the spark board.
To inject it on the johnny-five repl I used this code

var set_internal_RGB;
spark_board.on("ready", function(){
  var that = this;
  set_internal_RGB = function(red, green, blue){
    that.setInternalRGB.call(that, red, green, blue);
  }
})

board.on("ready", function() {
  this.repl.inject({
    setInternalRGB: set_internal_RGB
  });
});

@rwaldron
Copy link
Owner

I missed that patch landing and I'm not sure I would've accepted it without significant further discussion, as it's completely outside of the protocol and there was no new discussion here: voodootikigod/voodoospark#31 (aside from a few new posts).

  • Spark-IO follows the IO Plugin specification and it's not clear how this fits
  • I'm not sure it's a good idea to tamper with the built-in led, since it serves to provide color coded information about the board's state.
    • If user code changes the color, there is no way to coordinate changes to the color state that are made by board itself. User code may write "solid red", but the board may want to indicate something with "pulsing blue".
    • This seems analogous to tampering with the RX and TX leds on the Arduino, or Power Leds on the Edison (or any other board's special indicator light).

cc @Resseguie

@Resseguie
Copy link
Collaborator

@rwaldron I believe we talked about setting a special pin number in #31 to make it fit the IO Plugin spec?

I understand what you're saying on tampering with the built-in led, but I'm torn. It's just so darn bright. For example, it caused problems in the @cheerfulj5 project because it was brighter than the RGB I was using. Had to physically cover it. In that case, I just wanted to be able to turn it off. (Or alternatively use it for the color output instead of a second RGB.)

I'm not sure it would conflict with the board status colors as much as we first thought because the J5/spark-io code wouldn't be running anyway when anything but "pulsing cyan" is showing, correct? If the board craps out, it will go to red SOS signal regardless. And the other colors are for indicating connection status before we even connect with spark-io. Our handling here would be similar to the Spark's core RGBClass implementation, where I assume (thought haven't tested) that it gives you control during normal activity but the board takes over during boot up and special conditions.

@rwaldron
Copy link
Owner

@Resseguie All good points.

I still have concerns, mainly about the limited usefulness of this feature w/r to Johnny-Five. For example: users might assume that they can use the Led.RGB class to control this—that's not possible, since that class expects to operate on 3 signals.

I really oppose the special casing here, because I hate "scenario solving" and this is exactly that.

@Resseguie
Copy link
Collaborator

@rwaldron In general are IO Plugins limited to only the Johnny-Five defined specification, or could spark-io offer a function to control it (in an attempt to map to the Spark API) but that function just not be exposed in Johnny-Five since it's hardware specific?

@rwaldron
Copy link
Owner

or could spark-io offer a function to control it (in an attempt to map to the Spark API) but that function just not be exposed in Johnny-Five since it's hardware specific?

That's really the only thing we can do.

@rwaldron
Copy link
Owner

@canda

To inject it on the johnny-five repl I used this code

var set_internal_RGB;
spark_board.on("ready", function(){
  var that = this;
  set_internal_RGB = function(red, green, blue){
    that.setInternalRGB.call(that, red, green, blue);
  }
})
board.on("ready", function() {
  this.repl.inject({
    setInternalRGB: set_internal_RGB
  });
});

None of that is necessary, take a look:

var five = require("johnny-five");
var Spark = require("spark-io");
var board = new five.Board({
  io: new Spark({
    token: process.env.SPARK_TOKEN,
    deviceId: process.env.SPARK_DEVICE_ID
  })
});

board.on("ready", function() {

  this.repl.inject({
    rgb: function(r, g, b) {
      this.io.setInternalRGB(r, g, b);
    }.bind(this)
  });
});

@@ -337,6 +337,17 @@ Spark.prototype.pinMode = function(pin, mode) {
return this;
};

Spark.prototype["setInternalRGB"] = function(red, green, blue){
Copy link
Owner

Choose a reason for hiding this comment

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

Spark.prototype["setInternalRGB"] => Spark.prototype.internalRGB

@rwaldron
Copy link
Owner

This needs a test added

@Resseguie
Copy link
Collaborator

Does it need to behave like Led.RGB.color in that it returns the current color if no params passed? Is that even possible with the Spark API? Just thinking out loud.

@rwaldron
Copy link
Owner

@Resseguie my plan was to accept this as an initial commit and then extend the capabilities.

I plan to:

  • make it return the last color it received, if no args passed
  • accept an array [r, g, b] or hex string "#ffffff"

@canda
Copy link
Contributor Author

canda commented Mar 31, 2015

@rwaldron isn't it better to receive an object rather than an array?

{
  red: red_value,
  green: green_value,
  blue: blue_value
}

@rwaldron
Copy link
Owner

@canda the function will accept an array of byte values, a hex string or three distinct byte arguments. It will always return an object (that look like the object you illustrated)

@Resseguie
Copy link
Collaborator

@rwaldron then it looks fine to me. I've got some ideas to add to it as well later, but I want to think them through first.

@rwaldron
Copy link
Owner

rwaldron commented Apr 1, 2015

@Resseguie cool :)

rwaldron added a commit that referenced this pull request Apr 1, 2015
Added spark internal rgb support
@rwaldron rwaldron merged commit 0b4acce into rwaldron:master Apr 1, 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.

3 participants