Add option to round scaled sensor values

[fisher-alice]

This PR adds the option for scaled values from a sensor to be rounded . @dtex @islemaster

In the forked code-dot-org/johnny-five project, scaled values are always rounded as implemented in this PR. However, I wanted to allow the choice for scaled values to be returned as a float or an integer.

Thus, I added an optional boolean parameter rounded to the Sensor method scale with the default value of false so that current users of this method are not impacted by this change. I added the isScaledRounded as a Sensor instance property which is initialized to false. If true is passed by the parameter rounded then isScaledRounded is assigned true. Thus, the scaled value is returned as a rounded integer.

My initial approach is described above. but as suggested by @dtex , I added to the constructor options object an isScaledRounded property. It can be updated as a public property of the instance as shown by this unit test added by the PR.

Testing

I revised existing unit tests in test/board.js and test/sensor.js and added a unit test for when Sensor.scale when isScaledRounded is assigned to true.

Just a note that if this PR is merged, then code-dot-org/code-dot-org is able to get onto mainline johnny-five. For this particular customization, the only changes needed are to add an options property isScaledRounded when the light and sound sensors are initialized in PlaygroundComponents.js

[dtex]

Thanks for taking the time to submit this.

I think the first thing to ask here is why not just use Math.round() on the scaled value in user code? Looking at Johnny-Five examples, you'll see several occurrences where Math.round() is used in example programs. Johnny-Five has always been a gateway to JavaScript for many people and this is an excellent way to learn about built-ins.

That said, I hate that you have to keep a separate fork for code.org. What's the age range of your audience?

I have a few other, less subjective comments:

scale is deprecated in favor of scaleTo, but only the former was updated.

We've been trying to avoid adding optional parameters to method signatures since it's not very scalable. Instead, this is something I would expect to be set in the constructor options object and to be changed as either a public property of the instance or through a dedicated getter/setter.

If you pass true as the 3rd param to scale() and then call scale() again without the 3rd param it will disable rounding. That would be a surprise for the user since they didn't explicitly ask for rounding to be disabled.

[fisher-alice]

Thanks for taking the time to submit this.

I think the first thing to ask here is why not just use Math.round() on the scaled value in user code? Looking at Johnny-Five examples, you'll see several occurrences where Math.round() is used in example programs. Johnny-Five has always been a gateway to JavaScript for many people and this is an excellent way to learn about built-ins.

That said, I hate that you have to keep a separate fork for code.org. What's the age range of your audience?

I have a few other, less subjective comments:

scale is deprecated in favor of scaleTo, but only the former was updated.

We've been trying to avoid adding optional parameters to method signatures since it's not very scalable. Instead, this is something I would expect to be set in the constructor options object and to be changed as either a public property of the instance or through a dedicated getter/setter.

If you pass true as the 3rd param to scale() and then call scale() again without the 3rd param it will disable rounding. That would be a surprise for the user since they didn't explicitly ask for rounding to be disabled.

Thanks so much for the feedback! I will investigate a way so that we don't need this customization to scale. Our goal is to be able to get back on mainline and we're very close now.

[fisher-alice]

Thanks for taking the time to submit this.

I think the first thing to ask here is why not just use Math.round() on the scaled value in user code? Looking at Johnny-Five examples, you'll see several occurrences where Math.round() is used in example programs. Johnny-Five has always been a gateway to JavaScript for many people and this is an excellent way to learn about built-ins.

That said, I hate that you have to keep a separate fork for code.org. What's the age range of your audience?

I have a few other, less subjective comments:

scale is deprecated in favor of scaleTo, but only the former was updated.

We've been trying to avoid adding optional parameters to method signatures since it's not very scalable. Instead, this is something I would expect to be set in the constructor options object and to be changed as either a public property of the instance or through a dedicated getter/setter.

If you pass true as the 3rd param to scale() and then call scale() again without the 3rd param it will disable rounding. That would be a surprise for the user since they didn't explicitly ask for rounding to be disabled.

Hello @dtex - thanks again for your feedback. I did a little digging and found out that we have existing curriculum that expects the result of scale to be an integer. Here is an example of a student level from our 'Creating Apps with Devices - Circuit Playground ('23-'24)'. So it's important that we still support this customization.
I made a couple updates based on your suggestions including tweaking the unit tests. Would love a second look by you. Thanks so much!

[fisher-alice]

Hello @dtex - thanks again for your feedback. I did a little digging and found out that we have existing curriculum that expects the result of scale to be an integer. Here is an example of a student level from our 'Creating Apps with Devices - Circuit Playground ('23-'24)'. So it's important that we still support this customization.
I made a couple updates based on your suggestions including tweaking the unit tests. Would love a second look by you. Thanks so much!

Hi @dtex , we recently chatted about needing to upgrade serialport in our main code-dot-org repo, and so I'm circling back to this PR. If you have time, would appreciate another look from you. Thanks!