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

fixes #110 #112

Merged
merged 2 commits into from Feb 4, 2020
Merged

fixes #110 #112

merged 2 commits into from Feb 4, 2020

Conversation

dmiddlecamp
Copy link
Contributor

checks the prefix which can otherwise be undefined, and would prevent these functions from working as expected.

src/Particle.js Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Jan 29, 2020

Coverage Status

Coverage increased (+0.5%) to 41.163% when pulling 2dd6a63 on downloadBinaryFix into bde184c on master.

@dmiddlecamp
Copy link
Contributor Author

Hmm, now getting errors around "this.headers", these functions might have more issues than expected, looking into it.

@dmiddlecamp
Copy link
Contributor Author

Okay, got downloadFirmwareBinary working, and made compatible changes to 'downloadProductFirmware', but I haven't tested that other call yet.

@dmiddlecamp
Copy link
Contributor Author

Steps to Test:

Here's the code I'm running for an internal testing tool, things you'll need:

  • a test app
  • an access token
//my test app
void setup() {
    Particle.publish("status", "Empty app, no product");
}

void loop() { }

var express = require('express');
var router = express.Router();

var path = require('path');
var fs = require('fs');
var when = require('when');
var pipeline = require('when/pipeline');
var settings = require('../settings.js');
var Hogan = require('hogan.js');
var Particle = require('particle-api-js');

var data = req.body;
  try {
    console.log('hi!  I got... ', req.body);

    // WARNING: Hey!  YOU!  Don't run anything like this in an untrusted user facing capacity
    // this is just for a testing tool!

    // NOTE: if you were going to make it user facing, don't like, let them specify any old file.
    var filename = data.file;
    var filename_only = path.basename(filename);


    // Get and compile our template
    //

    var contents = fs.readFileSync(filename).toString();
    var template = Hogan.compile(contents);


    // Render our template
    //

    var template_params = data.template;
    var rendered_contents = template.render(template_params);

    // Save it in the temp folder
    //

    var new_path = path.join(__dirname, '../public/javascripts/tests/iota/firmware_temp');
    var new_filename = path.join(new_path, filename_only);
    fs.writeFileSync(new_filename, rendered_contents);


    var compileParams = {
      files: {},
      platformId: data.platformId,
      auth: settings.particle_access_token
    };
    compileParams.files[filename_only] = new_filename;


    var particle = new Particle();


    pipeline([
        function() {
          return particle.compileCode(compileParams);
        },

        function(data) {
          console.log("success ? ", data);
          var binary_id = data.body.binary_id;
          return particle.downloadFirmwareBinary({ binaryId: binary_id, auth: settings.particle_access_token});
        },

        function(data) {
          console.log("binary downloaded?  Got back ", data);
          res.send({ok: true, msg: "done!"});
        }
    ]).catch(function(ex) {
      console.log("pipeline error ", ex);

      res.send({ok: false, error: ex });
    });
    
  }
  catch(ex) {
    console.log("exception thrown during compile request handling ", ex);
  }

@busticated busticated self-requested a review January 29, 2020 21:47
Copy link
Contributor

@busticated busticated left a comment

Choose a reason for hiding this comment

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

ah, looks like you accidentally checked in package-lock.json - you'll need to revert that before merging.

https://github.com/particle-iot/particle-api-js/pull/112/files#diff-32607347f8126e6534ebc7ebaec4853d

@dmiddlecamp
Copy link
Contributor Author

okay, I think I removed the errant file, hopefully I didn't mess up the branch too bad

Copy link
Contributor

@busticated busticated left a comment

Choose a reason for hiding this comment

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

thanks - i'll get this merged and shipped ASAP 👍

@busticated
Copy link
Contributor

@dmiddlecamp apologies for the churn here - i had to tweak some stuff to accommodate testing given the current tooling, etc. do you mind trying these changes quick before i merge?

@busticated busticated merged commit 11c288d into master Feb 4, 2020
@busticated busticated deleted the downloadBinaryFix branch February 4, 2020 20:40
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.

None yet

3 participants