Skip to content

Commit

Permalink
More review notes and docs, see #317
Browse files Browse the repository at this point in the history
jonathanolson committed Aug 23, 2017

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent d6fd66c commit 8f54f7a
Showing 26 changed files with 94 additions and 52 deletions.
54 changes: 27 additions & 27 deletions doc/implementation-notes.md
Original file line number Diff line number Diff line change
@@ -2,29 +2,29 @@

This document contains miscellaneous notes related to the implementation of circuit-construction-kit-common. It
supplements the internal (source code) documentation, and (hopefully) provides insight into "big picture" implementation
issues. The audience for this document is software developers who are familiar with JavaScript and PhET simulation
development (as described in [PhET Development Overview] (http://bit.ly/phet-html5-development-overview)).
issues. The audience for this document is software developers who are familiar with JavaScript and PhET simulation
development (as described in [PhET Development Overview](http://bit.ly/phet-html5-development-overview)).

This repo is supposed to contain code that would be used in potentially any circuit construction kit offshoot.
However, since we don't have designs for some of the sims (such as AC), it is kind of like a kitchen sink **or** a
This repo is supposed to contain code that would be used in potentially any circuit construction kit offshoot.
However, since we don't have designs for some of the sims (such as AC), it is kind of like a kitchen sink **or** a
dedicated repo for the CCK-DC simulation.

First, read [model.md](https://github.com/phetsims/circuit-construction-kit-common/blob/master/doc/model.md), which
First, read [model.md](https://github.com/phetsims/circuit-construction-kit-common/blob/master/doc/model.md), which
provides a high-level description of the simulation model.

## Terminology

This section enumerates types that you'll see used throughout the internal and external documentation. In no particular
This section enumerates types that you'll see used throughout the internal and external documentation. In no particular
order:

* CircuitElement - a model object that can participate in a circuit, such as a Wire, Resistor, Battery, etc.
* FixedLengthCircuitElement - a CircuitElement that cannot be stretched out, such as a Battery or Resistor. The only
* FixedLengthCircuitElement - a CircuitElement that cannot be stretched out, such as a Battery or Resistor. The only
stretchy element is a Wire, so every CircuitElement that is not a Wire is a FixedLengthCircuitElement.
* Vertex - the circuit is organized as a graph, where the edges are CircuitElements and the vertices are Vertex
* Vertex - the circuit is organized as a graph, where the edges are CircuitElements and the vertices are Vertex
instances. After the circuit is solved for unknown currents, a voltage is assigned to each Vertex.
* Ammeter - this ammeter is a "non-contact" ammeter which can take readings of a current by measuring magnetic fields
(without touching the circuit)
* SeriesAmmeter - this ammeter is a CircuitElement which measures current flowing through it.
* SeriesAmmeter - this ammeter is a CircuitElement which measures current flowing through it.
* Circuit - this contains the CircuitElements and Vertices as well as logic for solving for the unknown voltages
and currents given the voltages sources and resistances.
* CircuitNode - this displays the Circuit and contains logic for dragging and connecting vertices to each other.
@@ -35,11 +35,11 @@ as resistors.

This section describes how this simulation uses patterns that are common to most PhET simulations.

**Model-view transform**: Many PhET simulations have a model-view transform that maps between model and view coordinate
**Model-view transform**: Many PhET simulations have a model-view transform that maps between model and view coordinate
frames (see [ModelViewTransform2](https://github.com/phetsims/phetcommon/blob/master/js/view/ModelViewTransform2.js)).
While the CircuitElements are treated as physical objects, the dimensions of the objects have no bearing on the physics
of the circuitry (aside from the resistivity of wires), hence the model and view coordinates are taken as the same, with
the origin at the center of the screen. (If you don't understand that, don't worry about it.). The layout reflows to
of the circuitry (aside from the resistivity of wires), hence the model and view coordinates are taken as the same, with
the origin at the center of the screen. (If you don't understand that, don't worry about it.). The layout reflows to
move control panels to the edges to maximize the available play area.

**Query parameters**: Query parameters are used to enable sim-specific features, mainly for debugging and
@@ -63,31 +63,31 @@ for details.
* Each node defines its own lifelike and schematic nodes internally, so nothing needs to be disposed or re-created when
the view type changes. Hence the "lifelike/schematic" radio buttons are not treated like independent scenes, but
rather a single view property.
* Likewise ChargeNode can render positive or negative charges, though they are cleared and re-added when the charge view
* Likewise ChargeNode can render positive or negative charges, though they are cleared and re-added when the charge view
is changed.
* The CircuitElementNode subtypes like BatteryNode, ResistorNode, etc have the same signature because they are invoked
dynamically from CircuitLayerNode.initializeCircuitElementType.
* The CircuitElementNode subtypes like BatteryNode, ResistorNode, etc have the same signature because they are invoked
dynamically from CircuitLayerNode.initializeCircuitElementType.
* View Layering: the CircuitLayerNode shows circuit elements, highlights, solder, and sensors. Each CircuitElementNode
may node parts that appear in different layers, such as the highlight and the light bulb socket. Having the light bulb
socket in another layer makes it possible to show the electrons going "through" the socket (in z-ordering). The
CircuitElementNode constructors populate different layers of the CircuitLayerNode in their constructors and depopulate
may have node parts that appear in different layers, such as the highlight and the light bulb socket. Having the light
bulb socket in another layer makes it possible to show the electrons going "through" the socket (in z-ordering). The
CircuitElementNode constructors populate different layers of the CircuitLayerNode in their constructors and depopulate
in their dispose functions.
* To attain reasonable performance on iPad2, some of the CircuitLayerNode child node layers have been implemented in
WebGL using `renderer:'webgl'`. This means all of the nodes must be rendered with solid-fill Rectangle (without rounded
corners or gradients), and images. Node.toDataURLImageSynchronous is used throughout these view layers to rasterize as
images.
* CircuitElementEditPanel (which appears at the bottom of the screen when a CircuitElement is selected) and
* To attain reasonable performance on iPad2, some of the CircuitLayerNode child node layers have been implemented in
WebGL using `renderer:'webgl'`. This means all of the nodes must be rendered with solid-fill Rectangle (without rounded
corners or gradients), and images. Node.toDataURLImageSynchronous is used throughout these view layers to rasterize as
images.
* CircuitElementEditPanel (which appears at the bottom of the screen when a CircuitElement is selected) and
ValueNode.js (which shows a text readout over an item when "values" is checked) use a similar pattern of containing
logic for the different kinds of CircuitElements. Other ways to solve this may have been:
(1) create subclasses of CircuitElementEditPanel and ValueNode specific to the types
(2) create abstract methods in CircuitElement or FixedLengthCircuitElement that can be called by CircuitElementEditPanel
and ValueNode.
It seems best to isolate the code relevant to each within its file rather than scattering it around, hence there are
type checks in those view classes. On the downside, when a new element type is added, these files will need to be
updated.
It seems best to isolate the code relevant to each within its file rather than scattering it around, hence there are
type checks in those view classes. On the downside, when a new element type is added, these files will need to be
updated.

This document was adapted from the [Implementation Notes for Function Builder](https://github.com/phetsims/function-builder/blob/master/doc/implementation-notes.md)

## Unit Tests
This simulation provides 60+ unit tests to ensure accuracy of the model for a variety of circuit topologies, please
This simulation provides 60+ unit tests to ensure accuracy of the model for a variety of circuit topologies, please
test by launching localhost/circuit-construction-kit-common/tests/qunit/unit-tests.html
8 changes: 4 additions & 4 deletions doc/model.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
The currents and voltage drops throughout the circuit are determined based on the voltage supplies and resistances.
Kirkhoff's laws are applied using the Modified Nodal Analysis strategy. Please see
The currents and voltage drops throughout the circuit are determined based on the voltage supplies and resistances.
Kirkhoff's laws are applied using the Modified Nodal Analysis strategy. Please see
https://www.swarthmore.edu/NatSci/echeeve1/Ref/mna/MNA2.html for details.

Wires are constrained to have >=1E-6 Ohms of resistance to avoid numerical errors or unsolvable circuits. Loops cannot
be built without wires, hence this constraint guarantees that each loop will have at least some resistance and be
be built without wires, hence this constraint guarantees that each loop will have at least some resistance and be
solvable.

Batteries can be ideal (no resistance) or have an internal resistance (in the 3rd screen of Circuit Construction Kit: DC).
Batteries can be ideal (no resistance) or have an internal resistance (in the 2nd screen of Circuit Construction Kit: DC).

Resistors such as the dollar bill, eraser and dog have a resistance of 10^9 Ohms.

1 change: 1 addition & 0 deletions js/model/Ammeter.js
Original file line number Diff line number Diff line change
@@ -49,6 +49,7 @@ define( function( require ) {
/**
* Restore the ammeter to its initial conditions
* @public
* @override REVIEW: I've been tagging @override tags, should it be discussed whether this for-sure standard/helpful
*/
reset: function() {
Meter.prototype.reset.call( this );
3 changes: 2 additions & 1 deletion js/model/Battery.js
Original file line number Diff line number Diff line change
@@ -59,7 +59,7 @@ define( function( require ) {

/**
* Get the properties so that the circuit can be solved when changed.
* @returns {Property[]}
* @returns {Property[]} REVIEW: Type of Properties? Property.<Circuit>?
* @override
* @public
*/
@@ -70,6 +70,7 @@ define( function( require ) {
/**
* @returns {Object} the attributes of the battery in a state object
* @public
* REVIEW: Duck typing looks good for these objects, but it should be documented where the spec is.
*/
attributesToStateObject: function() {
return {
1 change: 1 addition & 0 deletions js/model/FixedLengthCircuitElement.js
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@ define( function( require ) {
* @param {Vertex} endVertex
* @param {number} distanceBetweenVertices - in screen coordinates
* @param {number} chargePathLength - the distance the charges travel (in screen coordinates). see CircuitElement.js
* REVIEW: Distance traveled in screen coordinates (in the model?) Can model coordinates be used?
* @param {Tandem} tandem
* @param {Object} [options]
* @constructor
3 changes: 2 additions & 1 deletion js/model/LightBulb.js
Original file line number Diff line number Diff line change
@@ -151,7 +151,7 @@ define( function( require ) {
/**
* Get the properties so that the circuit can be solved when changed.
* @override
* @returns {Property[]}
* @returns {Property[]} REVIEW: Type of Properties? Property.<Circuit>?
* @public
*/
getCircuitProperties: function() {
@@ -162,6 +162,7 @@ define( function( require ) {
* Returns a serialized form of the properties that characterize this LightBulb
* @returns {Object}
* @public
* REVIEW: Duck typing looks good for these objects, but it should be documented where the spec is.
*/
attributesToStateObject: function() {
return {
1 change: 1 addition & 0 deletions js/model/Meter.js
Original file line number Diff line number Diff line change
@@ -29,6 +29,7 @@ define( function( require ) {
} );

// @public {Property.<Vector2>} - the location of the body of the meter
//REVIEW: Prefer Vector2.ZERO for the initial value (doesn't create new object)
this.bodyPositionProperty = new Property( new Vector2( 0, 0 ), {
tandem: tandem.createTandem( 'bodyPositionProperty' ),
phetioValueType: TVector2
3 changes: 2 additions & 1 deletion js/model/Resistor.js
Original file line number Diff line number Diff line change
@@ -81,7 +81,7 @@ define( function( require ) {
/**
* Get the properties so that the circuit can be solved when changed.
* @override
* @returns {Property[]}
* @returns {Property[]} REVIEW: Type of Properties? Property.<Circuit>?
* @public
*/
getCircuitProperties: function() {
@@ -92,6 +92,7 @@ define( function( require ) {
* Get the attributes as a state object for serialization.
* @returns {Object}
* @public
* REVIEW: Duck typing looks good for these objects, but it should be documented where the spec is.
*/
attributesToStateObject: function() {
return {
3 changes: 2 additions & 1 deletion js/model/SeriesAmmeter.js
Original file line number Diff line number Diff line change
@@ -36,7 +36,7 @@ define( function( require ) {
/**
* Get the properties so that the circuit can be solved when changed.
* @override
* @returns {Property[]}
* @returns {Property[]} REVIEW: Type of Properties? Property.<Circuit>?
* @public
*/
getCircuitProperties: function() {
@@ -49,6 +49,7 @@ define( function( require ) {
* Get the attributes as a state object for serialization.
* @returns {Object}
* @public
* REVIEW: Duck typing looks good for these objects, but it should be documented where the spec is.
*/
attributesToStateObject: function() {
return {
3 changes: 2 additions & 1 deletion js/model/Switch.js
Original file line number Diff line number Diff line change
@@ -89,7 +89,7 @@ define( function( require ) {
/**
* Get the properties so that the circuit can be solved when changed.
* @override
* @returns {Property[]}
* @returns {Property[]} REVIEW: Type of Properties? Property.<Circuit>?
* @public
*/
getCircuitProperties: function() {
@@ -100,6 +100,7 @@ define( function( require ) {
* Get the attributes as a state object for serialization.
* @returns {Object}
* @public
* REVIEW: Duck typing looks good for these objects, but it should be documented where the spec is.
*/
attributesToStateObject: function() {
return {
3 changes: 3 additions & 0 deletions js/model/Voltmeter.js
Original file line number Diff line number Diff line change
@@ -34,12 +34,14 @@ define( function( require ) {
} );

// @public {Property.<Vector2>} - the position of the tip of the red probe in model=view coordinates.
//REVIEW: Use Vector2.ZERO to avoid allocations (for the zero vector)
this.redProbePositionProperty = new Property( new Vector2(), {
tandem: tandem.createTandem( 'redProbePositionProperty' ),
phetioValueType: TVector2
} );

// @public {Property.<Vector2>} - the position of the black probe in model=view coordinates
//REVIEW: Use Vector2.ZERO to avoid allocations (for the zero vector)
this.blackProbePositionProperty = new Property( new Vector2(), {
tandem: tandem.createTandem( 'blackProbePositionProperty' ),
phetioValueType: TVector2
@@ -53,6 +55,7 @@ define( function( require ) {
/**
* Reset the voltmeter, called when reset all is pressed.
* @public
* @override
*/
reset: function() {
Meter.prototype.reset.call( this );
10 changes: 8 additions & 2 deletions js/model/Wire.js
Original file line number Diff line number Diff line change
@@ -47,10 +47,11 @@ define( function( require ) {
// @public {Property.<number>} - when the length changes the ChargeLayout must be run
this.lengthProperty = new NumberProperty( 0 );

// @private - batch changes so that the length doesn't change incrementally when individual vertices move
// @private {boolean} - batch changes so that the length doesn't change incrementally when individual vertices move
this.wireDirty = true;

// When the vertex moves, updates the resistance and charge path length.
//REVIEW: If it doesn't update something immediately, I like to think of it as marking it as dirty, instead of updating.
var updateWire = function() {
self.wireDirty = true;
};
@@ -65,6 +66,7 @@ define( function( require ) {
this.resistivityProperty.link( updateWire );

// @private {function} - for disposal
//REVIEW: Again if memory is an issue (I'll investigate), having this as a method may be better.
this.disposeWire = function() {
self.vertexMovedEmitter.removeListener( updateWire );
self.resistivityProperty.unlink( updateWire );
@@ -80,10 +82,12 @@ define( function( require ) {
/**
* Batch changes so that the length doesn't change incrementally when both vertices move one at a time.
* @public
* REVIEW: Would normally name this update(), since it has no DT and conditionally updates based on the dirty flag.
*/
step: function() {
if ( this.wireDirty ) {
var self = this;
//REVIEW: Another place where having a shortcut to the position properties (on the circuit element) would be nice.
var startPosition = self.startVertexProperty.get().positionProperty.get();
var endPosition = self.endVertexProperty.get().positionProperty.get();
var viewLength = startPosition.distance( endPosition );
@@ -101,7 +105,7 @@ define( function( require ) {
/**
* Get the properties so that the circuit can be solved when changed.
* @override
* @returns {Property[]}
* @returns {Property[]} REVIEW: Type of property?
* @public
*/
getCircuitProperties: function() {
@@ -111,6 +115,7 @@ define( function( require ) {
/**
* Releases all resources related to the Wire, called when it will no longer be used.
* @public
* @override
*/
dispose: function() {
this.disposeWire();
@@ -121,6 +126,7 @@ define( function( require ) {
* Returns an object with the state of the Wire, so that it can be saved/loaded.
* @returns {Object}
* @public
* REVIEW: Duck typing looks good for these objects, but it should be documented where the spec is.
*/
attributesToStateObject: function() {
return {
2 changes: 1 addition & 1 deletion js/view/BatteryNode.js
Original file line number Diff line number Diff line change
@@ -117,7 +117,7 @@ define( function( require ) {

/**
* Identifies the images used to render this node so they can be prepopulated in the WebGL sprite sheet.
* @public
* @public {Array.<Image>}
*/
webglSpriteNodes: [
new Image( batteryImage ),
Loading

0 comments on commit 8f54f7a

Please sign in to comment.