Hi, Here's a patch for a very old bug in the microwave inductor (which is a 5.1.0 milestone).
I haven't fixed the underlying issue here, but it does stop invalid outputs being generated with a warning. An example of inputs that generate invalid outputs (0.25mm trace width): * Draw inductor 5mm long, ask for 6mm: this is too small * Draw inductor 5mm long, as for 7mm: this can't be represented accurately These aren't trivial to fix and only affect very "Short" inductors of a few small turns. I have done the calculations for the first case (it doesn't have a closed-form solution), but haven't started the second bit, and there's a refactor and a good think needed. So, submitting this to close out the 5.1.0 bug and at least prevent invalid outputs. Cheers, John
From 37ffeb90c01dbd1a9c81e92333f993527f7fe517 Mon Sep 17 00:00:00 2001 From: John Beard <john.j.be...@gmail.com> Date: Thu, 24 Jan 2019 17:02:12 +0000 Subject: [PATCH] Pcbnew: Disallow invalid mwwave inductor lengths Some microwave inductor lengths cause invalid outputs for the calculations, which causes jagged outputs. * If the request length is such that four arcs and no straight segments is too long * If the length is such that an N-turn coil is too short, but an N+1-turn coil is too long. This can happen when the coil count is small. This patch doesn't fix the underlying geometric issue here - fixing the first requires a numerical method, and fixing the second probably needs an iterative approach. Both of these could benefit from a refactor. However, this patch does prevent the tools producing invalid outputs, which can sometimes be quite subtle mistakes if the "jags" are small. Fixes: lp:1792119 * https://bugs.launchpad.net/kicad/+bug/1792119 --- pcbnew/microwave/microwave_inductor.cpp | 60 ++++++++++++++++++++----- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/pcbnew/microwave/microwave_inductor.cpp b/pcbnew/microwave/microwave_inductor.cpp index d5a8d81c2..2136a54b5 100644 --- a/pcbnew/microwave/microwave_inductor.cpp +++ b/pcbnew/microwave/microwave_inductor.cpp @@ -79,6 +79,15 @@ static void gen_arc( std::vector <wxPoint>& aBuffer, } +enum class INDUCTOR_S_SHAPE_RESULT +{ + OK, /// S-shape constructed + TOO_LONG, /// Requested length too long + TOO_SHORT, /// Requested length too short + NO_REPR, /// Requested length can't be represented +}; + + /** * Function BuildCornersList_S_Shape * Create a path like a S-shaped coil @@ -88,10 +97,8 @@ static void gen_arc( std::vector <wxPoint>& aBuffer, * @param aLength = full length of the path * @param aWidth = segment width */ -static int BuildCornersList_S_Shape( std::vector <wxPoint>& aBuffer, - const wxPoint& aStartPoint, - const wxPoint& aEndPoint, - int aLength, int aWidth ) +static INDUCTOR_S_SHAPE_RESULT BuildCornersList_S_Shape( std::vector<wxPoint>& aBuffer, + const wxPoint& aStartPoint, const wxPoint& aEndPoint, int aLength, int aWidth ) { /* We must determine: * segm_count = number of segments perpendicular to the direction @@ -171,7 +178,7 @@ static int BuildCornersList_S_Shape( std::vector <wxPoint>& aBuffer, if( radius < aWidth ) // Radius too small. { // Unable to create line: Requested length value is too large for room - return 0; + return INDUCTOR_S_SHAPE_RESULT::TOO_LONG; } } @@ -192,6 +199,30 @@ static int BuildCornersList_S_Shape( std::vector <wxPoint>& aBuffer, // reduce len of the segm_count segments + 2 half size segments (= 1 full size segment) segm_len -= delta_size / (segm_count + 1); + // at this point, it could still be that the requested length is too + // short (because 4 quarter-circles are too long) + // to fix this is a relatively complex numerical problem which probably + // needs a refactor in this area. For now, just reject these cases: + { + const int min_total_length = 2 * stubs_len + 2 * M_PI * ADJUST_SIZE * radius; + if( min_total_length > aLength ) + { + // we can't express this inductor with 90-deg arcs of this radius + return INDUCTOR_S_SHAPE_RESULT::TOO_SHORT; + } + } + + if( segm_len - 2 * radius < 0 ) + { + // we can't represent this exact requested length with this number + // of segments (using the current algorithm). This stems from when + // you add a segment, you also add another half-circle, so there's a + // little bit of "dead" space. + // It's a bit ugly to just reject the input, as it might be possible + // to tweak the radius, but, again, that probably needs a refactor. + return INDUCTOR_S_SHAPE_RESULT::NO_REPR; + } + // Generate first line (the first stub) and first arc (90 deg arc) pt = aStartPoint; aBuffer.push_back( pt ); @@ -257,7 +288,7 @@ static int BuildCornersList_S_Shape( std::vector <wxPoint>& aBuffer, // push last point (end point) aBuffer.push_back( aEndPoint ); - return 1; + return INDUCTOR_S_SHAPE_RESULT::OK; } @@ -298,7 +329,6 @@ MODULE* MWAVE::CreateMicrowaveInductor( INDUCTOR_PATTERN& inductorPattern, */ D_PAD* pad; - int ll; wxString msg; auto pt = inductorPattern.m_End - inductorPattern.m_Start; @@ -324,14 +354,22 @@ MODULE* MWAVE::CreateMicrowaveInductor( INDUCTOR_PATTERN& inductorPattern, // Calculate the elements. std::vector <wxPoint> buffer; - ll = BuildCornersList_S_Shape( buffer, inductorPattern.m_Start, - inductorPattern.m_End, inductorPattern.m_length, - inductorPattern.m_Width ); + const INDUCTOR_S_SHAPE_RESULT res = BuildCornersList_S_Shape( buffer, inductorPattern.m_Start, + inductorPattern.m_End, inductorPattern.m_length, inductorPattern.m_Width ); - if( !ll ) + switch( res ) { + case INDUCTOR_S_SHAPE_RESULT::TOO_LONG: aErrorMessage = _( "Requested length too large" ); return nullptr; + case INDUCTOR_S_SHAPE_RESULT::TOO_SHORT: + aErrorMessage = _( "Requested length too small" ); + return nullptr; + case INDUCTOR_S_SHAPE_RESULT::NO_REPR: + aErrorMessage = _( "Requested length can't be represented" ); + return nullptr; + case INDUCTOR_S_SHAPE_RESULT::OK: + break; } // Generate footprint. the value is also used as footprint name. -- 2.20.1
_______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
