From 33d07bed5890bf19eb7a73188ab24858818a1195 Mon Sep 17 00:00:00 2001 From: Christian Manivong Date: Fri, 21 Aug 2026 13:07:37 +0700 Subject: [PATCH] fix: inherit PhoneDriver, and stop asserting a contract the driver rejected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes netork#117. A Yealink is a desk phone. It inherited AccessPointDriver because nothing better existed, and netOrk reads that role to populate its wireless page, its AP profile pickers and its SSID drift view — so a phone appeared in all three. napalm-device-types now has PhoneDriver. TYPE_LABEL comes from it, so the local override is gone. The two failing fingerprint tests were asserting the opposite of a deliberate decision. HTTP_FINGERPRINT carries "yealink" as a heavy but *not* mandatory rule, with the reason written next to it: some models answer / with a redirect to their API page, whose body says "yealink" nowhere, and a mandatory rule disqualifies the driver whenever its pattern is absent. Making it mandatory would rule out a real phone. The tests demanded it be mandatory, and the Cisco test then asserted `not all(...)` over an empty list, which is False. Both now check the property that actually protects against a false match: none of this driver's patterns appear on a Cisco page, so it contributes no score and never wins. --- .../__pycache__/yealink.cpython-312.pyc | Bin 11029 -> 10971 bytes napalm_yealink/yealink.py | 5 ++- ...t_fingerprint.cpython-312-pytest-9.0.3.pyc | Bin 8317 -> 0 bytes tests/test_fingerprint.py | 33 +++++++++++++----- 4 files changed, 27 insertions(+), 11 deletions(-) delete mode 100644 tests/__pycache__/test_fingerprint.cpython-312-pytest-9.0.3.pyc diff --git a/napalm_yealink/__pycache__/yealink.cpython-312.pyc b/napalm_yealink/__pycache__/yealink.cpython-312.pyc index dee92707b4753e99ec3c8f790a49cd44ae82ef5f..8da1279e52852452b6a44f1b111cca0596a5b85e 100644 GIT binary patch delta 2985 zcma)8du&rx81HSnuIt&O+q$)D*R|a`y1OTH8+*Wj!Jc5sTqa~yXv$7E3R}CJTOOOl z%{YRh5#~`a8WSS;;E@rXCI%8C35i6bA<+c9puxXPH1Qwd4@pe?zT2(s7%pwo-|su$ z_xrx{=(*?Iw|CxsHtV~rEQ1O@@BMLn|HZki(?ZUt=g+9^Io+Y~Xpl{?P&hhpa4aa3 z{(X^fu#<%j1sUI?sS1tI_Pc=0Cvi_19i>XwKCxaRu^NFxX}&DlNb_g4zHDF?Y*rY2$rG;-KR-u3o5qpaj95V4gy`!hk>+kOv?2T0{M4b!4 zjF5(qj)0wtY0SpwNrhnNw~1ZoUU*0v)Ona9>X_0)6-uqVExRn&2BstcIGG*2%{-iK z5q2#c&)zQR8qm|o@8r~o%50jky$(QBGfb4VQe^mmgxl4*nBT^0jW%I;VVm)qM%Re? zI!;V>A&ZNq!MqMkGDIXlJOEM%rk&4Eni{5+K`Ap)=4EBYzRrAuoMl1 zhl8?~hK8fcNR%1Nx}9C#&K(1yesNf#@JNK!D!pTm=@Ws;u}FY2?4NBx!1+je0H@QL z4nG_}?=A2M;|mi7?+BSK7++i$ZZg;B&w=A_OqfyK%XZz;yO@K|+pDv7f{X0}5Vf%E zb7oxjt7ehBg?=A`ga2i&E{yBgveIiU&E(8-sqM*95XokK-(u#sEFSXi@|^!G-&xd5 z-cL<>v#5cbO->Ym9>0e+e9v>CY>p6AXie7zG<%`pQWVaN9`W+MUr&o_W0di1KXpZq=tN% zq+4Z5CrjGNwIu0`%f$aIv3L@Trct&$n*@=pJ8(F_MkPrDjo6N^bl8nwCGx=<*6=y2 zCu!S40sfl93FEr#uo(HqcPq!dyH)s&=ayF}$F!`GqAYe#e1KWX^`lsY(3Cas zT(V-mx4dflJcd66fF(tHDhJQi=ODe>xi=CS+r8w^PWU^*lS^*akEt426Brxo?uNtn zd90`Ov>Pm0i#6CDbbgGWbf@G9tP>p5aAY39fRd7kYFT)hSJsY&r0C*U*e4JZHA;a5 zKNd0n5rz=_T=v*?V`xVBzaESGD4H)K zyo7KJ;RM3#2yY?{N>z`cfX<)tALEreqDNqgbioK^j- z9lFm?SCngh($_6~UC|`8DSA|i`D`9YHu(J`2crjB(C=q>WHArIdJO5JlN0dr9Q$kB zysq+C+2l>;u16!3p4$FUg{)-#2l(ZN AAOHXW delta 3246 zcma)9drVtp6u-B!fYBc_9qA@=H@GsA~g|=j7DTLoS_nhB3 z=ljn69+&TLchX@-Jn@>o)3hpn5IKI1v=8BXOCo^XnV@QRgK6oFV38xa(d*c3YviG9YRI4qp% zBm!{|=XuvTE0*Lq(Ek!q!k)F|an&dHIj zh?oA7ep0(^Q`}@5@h%HW1;{#(m1H~dfvh3}q#k57*+CjW){v{nAZdgM4_KPOQVY@# z(o1$MkpL)lU}y&E0~rKaPlm|Vqy<76K-)=LL2D$tNE`T?fD;1ghv#;X0eJ2J*-VDX zZW0D&5GE%B_pH@JZ)r@jFC;Cg-oq``FfkuPWF)q&^n;hIN1!&FwCe2 zES(^)04YPMUXWdo_)3tQK=y&`RwHCT*|IF0%k`d9HZOSOYuL56xM-W0O{Y|L!L~ax zC0h*fgsRQVW)0_jJVoN!G+kJ<=~^a|oymoIcF8=g1bx*hX86il(5M%o4#CGBv@?Fo zs^9UFr6zBNw8CwAItA{pT|1W=k=X!Xh_KqU=>@|+8`o!(+O39lo@_NU8KRm>q={-c zXVu$@HlylUL&&7{EN#L-DR$uC*myKHF+8;|SKfz7+YsszYzTG)9HDHZeh`K|b0Rj& z9+#T=X0{^v`LWe6r9MkF#>yf!#}&btCgyRsbzz7OtDXzeX0#3fEY=)R<4G+wm)k0v z6Unr$9$t;Rui?e5Na|;ARzyb3wjiHw2aqkf@`9dD%;~t0gKH@VL0|N4OsVNHsBl~* ztD(yGtf79)(hFeNR7%s-aAxp@MdS0TVVj2*C8?WPJyX?tkmE}0F=(o;LAPs}=IP6I z+w6(iIah2vI(jhIxO~qHm!@keJsVFYR6`(IB5RJ@?42PV8j0>bI4L{UCZVOK)AWk` zsO*{eLNXmE6zkKy2snG)0T2uZ?eK@QWv#VgKDD~P_F2Ac2m03<%YO6r`kny8pBQkG z`@!9E(a}O%;2|Nf>wMwz!=Tb50J1>+Y_6^oUfz>+vh)<%Z$fBcN9()2c^=)6-{spT zJzE;}QQscvxzebC`fbwb(x_+Z`=l33qqa5-NG}&rR{|b()aNT|0jwnULW37t@K(c^ z^y;PDK)wy@tIm3yPUjaLn{4cnUM~u}Oy|C6+$misjjA;DNpF-!HJW;))zYYk{Nc*C z3RKYkcS(HYk3c_u_eZ363*^f!2`w-ryv6Gq04RSKIIT*%s#IPok+G5qla4>zmF}gQDqO-XX8IEZ! zsitt>i-~k3?T zekhGv9qN^SDxyY1Tf|&9*aIOvq*g)${4ea+P^XLsqG5&0D|c6!Tim=aP0eR|;aiIa z*kJpnrB^U1Zf$5O>WdY4BHsw%-N9Sa>EzApc6Y%(oLX2n(@6}q8rFC+IXVi*_AQvt ztP}&y5HJTlipH}DW;rv4r=wt4f^&8r9cDr+m+i*=CnHY&37~VZ{fL_n8 z4EOJHg4d|rH#Id88y%01jZ98Vjz_2HHmtT8VFDqD(1$R9Fo=)-yO|gq% zUuXf3BO!XpTeB0R>*I-7>+^Pn42A{p29c{MwmCiDzg&celrBs_Cy**Y|V zqHA#ZE_2@rM}DsIo!tK|i*>f!-U)r<*tvRt=T?4)8HN#Z4aHO4aK&QN$Fj$$8jDdp z3aN}Rgf20=fJa~MZFR7J diff --git a/napalm_yealink/yealink.py b/napalm_yealink/yealink.py index 4e848a6..97f4237 100644 --- a/napalm_yealink/yealink.py +++ b/napalm_yealink/yealink.py @@ -21,14 +21,13 @@ import requests from requests.exceptions import RequestException from napalm.base.exceptions import ConnectionClosedException, ConnectionException -from napalm_device_types import AccessPointDriver, FingerprintRule +from napalm_device_types import FingerprintRule, PhoneDriver from napalm_device_types.models import HealthMetricsDict -class YealinkDriver(AccessPointDriver): +class YealinkDriver(PhoneDriver): """NAPALM driver for Yealink IP phones (T-series, W-series, CP-series, VP-series).""" - TYPE_LABEL = "Phone" VENDOR = "Yealink" DRIVER_NAME = "yealink" diff --git a/tests/__pycache__/test_fingerprint.cpython-312-pytest-9.0.3.pyc b/tests/__pycache__/test_fingerprint.cpython-312-pytest-9.0.3.pyc deleted file mode 100644 index 58496721c86c71d4ed4776ac4f184f1190166e79..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 8317 zcmeHNU2NOd6(%K0md!-|OSYuR7PhhBs7>VfH?nJI`LWVCO%>1eiq%*_C?ajsmMHa- zb|Oy#f-dOV!b8)pY4)&(JZylCY`|Uy4A{d4>}`9IEO}nJ4H&kEZEr*T(&TC9T>jct z9It7Ctt&wt9v+@^c}d>y<2{G^-QK+o3@kVQ@cis>fMNcIJNDwUlnoV>JB-9kG7>BK z((I&L=^zK9_FD@RK~~ft$?a9w`WNuT%#zEQLTe zO7$R{qy~`95)ZOP+5@sR?~CkRY`dVS(=y2sMb!me)^tsn$`avKIhj_}xe=l)$Rzqt z>|!F~Gnz)^1tld<RDkw*}UYwYP7u5>sUIOeC0C>`Q8zOmw0BST5fi(Hg#ODZ(%{os4M5(9#vr zj#EQ-M2j>OjRr;2=C7vG5KRE948Z{#;S7H+e-cF(if$CWkp_a}(RDNSoXO?ItMf`) zS5(airn9MJS~D8Jir5m9#5f>oGuipHBoaAC#OZYQYBDXV$&75&i&}C@MzawIgvMk-$|)}N^#0%h-fZvJVUR<@)H$J zgtmYaL>)*$lqg}4FTv^Y&S>8QafA62zyE_wwY{&x_m{Z-Rla|nbJsPle`(M(R9w~Z zSmpXZ$T){>)q5Bkc8)uGmFq8UJ`ebmx&9645L26zegdN$I0+r-GV9>Ww!qi~1}?G% zrohe}gFRotr~O>VQ9_ql7`HQII&3263HH2bBXR9EmTEZyTg*mloY3)`ICJ9~@RzRni$ z=PR;+@le7DjLl?Kd4+xG8y=3Zi`@ulz_E#SP+Y7>Cte+kikr;FBVIQ8c1x zLV>JC@KRR-Ug&mtcX~-HIC#O_@7;-M*aHC zb^d6XKU(6Vcdvmc@kj4oEA!EH4%aoyVD34x%te=m&Ap1NR;jaTUd84de@6iq@?|*j z7YKu!0E<0#|925qXU*owQ*b!|vFHvEdnKM2xiY>TUgR*ulCPpTf~{CM3Yz{DL*!{Z zx+7}HGvKmy{PT4Do9{6495^7{ykq}JnLkqEj^7;sQR0u>9Vqk1*Ew9*EQ7h{$TD{v z$3A*gT(wG_P4g-?=lDAcxO_48J@3!Y!PvKEIriA^**;{2c{czJS8#2?~qNUD2Lsf3z#w3%{OIy(fEYPz>(?ikA~Fjfq#zU5cNd z5JyMEvGMqY(Qob=9{X#CKM_#&QJ`pQCMRYUNz4&>O1VzD0Vp_Y(XNBA=1ftTBbB&# zsXLAm^zL|>kFRsMuGtoI*O6r|j(9lqp#I`U;B%lvPHd!T4Z%;I}ad;zfpq)U&|61}BB|lAWb4 zn}x(+%K0IChh>?i{T!2Lf9k)+MttN1_$5|Yqb`@!b(vscz!yDfu*9kY&_v;3{m`_m zLivE4UL2$#n9=o|H@6p|I3OkUEXj-3YcD!v)-O&+o!GCyxwL~I{=s~FgINs^EI<9$ z>+j~@&i`O^t+Blv9(bVPsC(n!A@<>MPYSg|{lzM~_>*ehn}!igVj{emaW!3>m>3f; zj3!=)kB^OyCMJw}r?=#JaP-U^akSM+x*`D*!$YT097TaonBXJbI;rf~-U>j;!*?^ZYihx=<$o8oa30v#prcPfEp|cV$*3hr%OC!H z&_gYC^EePYtV~Uq=!*%oF47?=8DT=v<+SkEH{TZ=$Os)4!aD?=C{t6qus9!Ge9AHk zI>hI&t^!pCby`uc3y)^5+pNP~P}z7CcfIXfu$gNgFc*W&wZbMyA9Nc)e+o7foJI)h zCs!3!mNeu?LAXK5iwwZdVjo4Nry2>2rKe_OO@t1Mx}vA$8jRHjp&I6{A6t|aTW+=} z>teOBtsIU$*g(qq&VfTLK`u7@P|G1hpe0sJ`XR%xWAerY)Z8rfkz5meB-fA;XhK6I z!rc{wGiDjbEUNB!ZR8@xVDJ(MVBfzqK6m5d&Xn`VaBNwrhGQizhLjVl@Tivg*gEH~ zYk1H!R9v;n#UQ{muY$fg2JGPLrJN6&&kNi#2ckCRRFQImlyZWf3FQR;6Uy1c6uANi zbDIAr;tA|}P7|=_L-rPGuatOzeEdaj3;CRZ@pxX*QrQzP%Rn}Qtm-qr`B7RI=43_n zaLr~88Z&1r!jwFd)}xCDQ`mx_|<%CW$0&- zYRgcG8(PvzEkl(?R7)UvRLcC&I_Iuy=w%uzu3CkF7Rzas276;bhp;XD(~kVp&PDba zb)qO`Q=({u>~C?Dw2gzJ`5`W9-g@|o7A5jE0)y75cT7f|Wuuv<(Q1Af^M2;i=r=)v zuMHz?9kyPc=C?rn{zWVCv?NK3F*M1f6Glqp^tP@rH0d)Z=%JxZR)R0+r^z=U8dC)g zAN9ZdEX#hxJo|U}`v9}foc%L%?0%4CPu&l(Y$u8nD7sNxVDE=~?BM+djvd$tbL=o} F{om{wAL0N2 diff --git a/tests/test_fingerprint.py b/tests/test_fingerprint.py index 0808f94..43e99dd 100644 --- a/tests/test_fingerprint.py +++ b/tests/test_fingerprint.py @@ -1,6 +1,6 @@ """Fingerprint tests for YealinkDriver.""" -from napalm_device_types import DeviceTypeDriver, FingerprintRule +from napalm_device_types import DeviceTypeDriver, PhoneDriver, role_keys_of from napalm_yealink import YealinkDriver @@ -24,9 +24,18 @@ def test_snmp_oid_prefix(): assert YealinkDriver.SNMP_OBJECT_ID_PREFIX == "1.3.6.1.4.1.37403" -def test_http_fingerprint_mandatory_yealink(): - mandatory = [r for r in YealinkDriver.HTTP_FINGERPRINT if r.mandatory] - assert any(r.pattern == "yealink" for r in mandatory) +def test_yealink_pattern_is_decisive_but_not_mandatory(): + """A mandatory rule disqualifies the driver whenever the pattern is absent. + + Some Yealink models answer / with a redirect to their API page, whose body + carries no "yealink" anywhere — making the rule mandatory would rule out a + real Yealink phone. It carries decisive weight instead, so a page that does + say "yealink" still wins by a wide margin. + """ + yealink_rules = [r for r in YealinkDriver.HTTP_FINGERPRINT if r.pattern == "yealink"] + assert yealink_rules, "the vendor pattern must be present at all" + assert all(not r.mandatory for r in yealink_rules) + assert max(r.weight for r in yealink_rules) >= 10.0 def test_fingerprint_matches_t58_title(): @@ -36,8 +45,16 @@ def test_fingerprint_matches_t58_title(): assert all(r.pattern in combined for r in mandatory) -def test_fingerprint_does_not_match_cisco(): - """Cisco-Seite enthält kein 'yealink' → mandatory Pattern fehlt.""" +def test_fingerprint_scores_nothing_on_a_cisco_page(): + """Without a mandatory rule, what keeps a Cisco phone out is that none of + Yealink's patterns match it at all — so the driver contributes zero score + and never wins.""" combined = "cisco ip phone cp-8841 " - mandatory = [r for r in YealinkDriver.HTTP_FINGERPRINT if r.mandatory] - assert not all(r.pattern in combined for r in mandatory) + assert not any(r.pattern in combined for r in YealinkDriver.HTTP_FINGERPRINT) + + +def test_declares_the_phone_role_not_access_point(): + """A desk phone used to inherit AccessPointDriver for want of anywhere + better, which put it in netOrk's wireless page and AP profile pickers.""" + assert issubclass(YealinkDriver, PhoneDriver) + assert role_keys_of(YealinkDriver) == ["phone"]