WebGuide's Filament Dialog Fix to ensure filaments inheritance chain is checked properly (#15855)

* fix: resolve filament vendor/type across split base presets

WebGuide's filament list dropped presets whose vendor and type came from different ancestors, and looped forever on inherits cycles. Walk the full inherits chain with a path-scoped guard, fill only missing values, and skip presets that still lack vendor or type. Add tests for split-base resolution and validate the real profile tree.

* Merge branch 'main' into feat/filament_dialog_fix

* Fixes unit test

* Merge branch 'main' into feat/filament_dialog_fix

* Merge branch 'main' into feat/filament_dialog_fix

* revert tests/fff_print/test_gcodewriter.cpp changes
This commit is contained in:
Lam Wei Lun
2026-09-23 20:23:52 +08:00
committed by GitHub
parent a8f1061a02
commit 79ea7cba27
3 changed files with 85 additions and 31 deletions
+45
View File
@@ -405,6 +405,20 @@ class TestTripleResolution(unittest.TestCase):
self.assertEqual(afi.resolve_triple("MyPLA @P1", fmap, {}), self.assertEqual(afi.resolve_triple("MyPLA @P1", fmap, {}),
("MyVendor", "PLA", "MyPLA")) ("MyVendor", "PLA", "MyPLA"))
def test_split_vendor_and_type_bases_resolve(self):
# A partial base is normal, not an error: vendor and type may live on
# different ancestors, with an intermediate supplying neither (the
# Snapmaker shape). The pair is complete at the instantiated preset.
recs = [
self.rec("APLA @P1", inherits="mid"),
self.rec("mid", inherits="typebase"),
self.rec("typebase", filament_type=["PLA"], inherits="vendorbase"),
self.rec("vendorbase", filament_vendor=["AV"]),
]
fmap = {r["name"]: r for r in recs}
self.assertEqual(afi.resolve_triple("APLA @P1", fmap, {}),
("AV", "PLA", "APLA"))
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# checks on synthetic trees # checks on synthetic trees
@@ -417,6 +431,24 @@ class TestChecks(OfCleanTreeCase):
self.assertNotIn("[ERROR]", out) self.assertNotIn("[ERROR]", out)
self.assertNotIn("[WARNING]", out) self.assertNotIn("[WARNING]", out)
def test_instantiated_preset_over_partial_bases_is_silent(self):
# Vendor and type split across two non-instantiated bases, an
# intermediate base with neither: base profiles are allowed to be
# partial. Only the instantiated preset must resolve both.
self.t.write_preset("VendorA", preset("XPLA vendorbase", instantiation=False,
filament_vendor="XV"))
self.t.write_preset("VendorA", preset("XPLA typebase", instantiation=False,
filament_type="PLA",
inherits="XPLA vendorbase"))
self.t.write_preset("VendorA", preset("XPLA mid", instantiation=False,
inherits="XPLA typebase"))
self.t.write_preset("VendorA", preset(
"XPLA @P1", inherits="XPLA mid",
filament_id=afi.generate_filament_id("XV", "PLA", "XPLA"),
compatible_printers=["P1"]))
errors, out = self.t.check()
self.assertEqual(errors, 0, out)
def test_check1_unknown_non_of_id(self): def test_check1_unknown_non_of_id(self):
self.t.write_preset("VendorA", preset("BPLA @base", filament_id="BOGUS_9", self.t.write_preset("VendorA", preset("BPLA @base", filament_id="BOGUS_9",
instantiation=False, instantiation=False,
@@ -1523,6 +1555,19 @@ class TestRealTree(unittest.TestCase):
self.assertEqual(analysis["missing_effective"], []) self.assertEqual(analysis["missing_effective"], [])
self.assertEqual(analysis["read_errors"], []) self.assertEqual(analysis["read_errors"], [])
def test_every_instantiated_filament_resolves_vendor_and_type(self):
# The property the web guide resolves at load: a partial base is fine as
# long as the instantiated preset ends up with both fields. Guards the
# split-base bundles (Snapmaker, Anker, SeeMeCNC).
analysis = afi.analyze_tree(REAL_PROFILES)
unresolved = [
(vendor, rec["name"], rec["triple"][0], rec["triple"][1])
for vendor, filaments in analysis["vendors"].items()
for rec in filaments.values()
if rec["instantiation"] and not (rec["triple"][0] and rec["triple"][1])
]
self.assertEqual(unresolved, [])
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# review-fix regressions # review-fix regressions
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
+20 -15
View File
@@ -1159,10 +1159,25 @@ bool GuideFrame::run()
} }
int GuideFrame::GetFilamentInfo( std::string VendorDirectory, json & pFilaList, std::string filepath, std::string &sVendor, std::string &sType) int GuideFrame::GetFilamentInfo( std::string VendorDirectory, json & pFilaList, std::string filepath, std::string &sVendor, std::string &sType)
{
std::unordered_set<std::string> visiting;
return GetFilamentInfo(VendorDirectory, pFilaList, filepath, sVendor, sType, visiting);
}
int GuideFrame::GetFilamentInfo(const std::string& VendorDirectory, json& pFilaList,
const std::string& filepath, std::string& sVendor,
std::string& sType, std::unordered_set<std::string>& visiting)
{ {
//GetStardardFilePath(filepath); //GetStardardFilePath(filepath);
BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " GetFilamentInfo:VendorDirectory - " << VendorDirectory << ", Filepath - "<<filepath; BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " GetFilamentInfo:VendorDirectory - " << VendorDirectory << ", Filepath - "<<filepath;
// Path-scoped guard: without it an `inherits` cycle would recurse forever.
// The repeated file was not inserted, so the cycle hit has nothing to erase.
if (!visiting.insert(filepath).second) {
BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " inherits cycle at " << filepath;
return -1;
}
// Resolve this file's own vendor/type into LOCAL variables, independent of // Resolve this file's own vendor/type into LOCAL variables, independent of
// whatever the caller already accumulated. The cache entry for `filepath` // whatever the caller already accumulated. The cache entry for `filepath`
// must reflect only this file's own inherits chain — passing the caller's // must reflect only this file's own inherits chain — passing the caller's
@@ -1197,7 +1212,6 @@ int GuideFrame::GetFilamentInfo( std::string VendorDirectory, json & pFilaList,
else else
BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << filepath << " - Not Contains filament_type"; BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << filepath << " - Not Contains filament_type";
if (vendor == "" || type == "") {
if (jLocal.contains("inherits")) { if (jLocal.contains("inherits")) {
std::string FName = jLocal["inherits"]; std::string FName = jLocal["inherits"];
@@ -1213,8 +1227,8 @@ int GuideFrame::GetFilamentInfo( std::string VendorDirectory, json & pFilaList,
inherits_path = (boost::filesystem::path(m_OrcaFilaLibPath) / boost::filesystem::path(FPath)).make_preferred(); inherits_path = (boost::filesystem::path(m_OrcaFilaLibPath) / boost::filesystem::path(FPath)).make_preferred();
if (boost::filesystem::exists(inherits_path)) { if (boost::filesystem::exists(inherits_path)) {
// Recurse with this file's own (vendor, type) as the chain accumulator. // Traverse the full chain for errors; inherited values only fill local gaps.
status = GetFilamentInfo(VendorDirectory, pFilaList, inherits_path.string(), vendor, type); status = GetFilamentInfo(VendorDirectory, pFilaList, inherits_path.string(), vendor, type, visiting);
} else { } else {
BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " inherits File Not Exist: " << inherits_path; BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " inherits File Not Exist: " << inherits_path;
status = -1; status = -1;
@@ -1222,16 +1236,6 @@ int GuideFrame::GetFilamentInfo( std::string VendorDirectory, json & pFilaList,
} }
} else { } else {
BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << filepath << " - Not Contains inherits"; BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << filepath << " - Not Contains inherits";
if (type == "") {
BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << "sType is Empty";
status = -1;
} else {
if (vendor == "")
vendor = "Generic";
status = 0;
}
}
} else {
status = 0; status = 0;
} }
} }
@@ -1250,6 +1254,7 @@ int GuideFrame::GetFilamentInfo( std::string VendorDirectory, json & pFilaList,
// Merge this file's resolved values into the caller's out-params (fill empties only). // Merge this file's resolved values into the caller's out-params (fill empties only).
if (sVendor.empty()) sVendor = vendor; if (sVendor.empty()) sVendor = vendor;
if (sType.empty()) sType = type; if (sType.empty()) sType = type;
visiting.erase(filepath);
return status; return status;
} }
@@ -1803,8 +1808,8 @@ int GuideFrame::LoadProfileFamily(std::string strVendor, std::string strFilePath
std::string sT; std::string sT;
int nRet = GetFilamentInfo(vendor_dir.string(),tFilaList, sub_file, sV, sT); int nRet = GetFilamentInfo(vendor_dir.string(),tFilaList, sub_file, sV, sT);
if (nRet != 0) { if (nRet != 0 || sV.empty() || sT.empty()) {
BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << "Load Filament:" << s1 << ",GetFilamentInfo Failed, Vendor:" << sV << ",Type:"<< sT; BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << "Load Filament:" << s1 << ",unresolved vendor/type, Vendor:" << sV << ",Type:"<< sT;
continue; continue;
} }
+4
View File
@@ -33,6 +33,7 @@
#include <atomic> #include <atomic>
#include <memory> #include <memory>
#include <unordered_map> #include <unordered_map>
#include <unordered_set>
#include <nlohmann/json.hpp> #include <nlohmann/json.hpp>
@@ -110,6 +111,9 @@ public:
void on_dpi_changed(const wxRect &suggested_rect) {} void on_dpi_changed(const wxRect &suggested_rect) {}
private: private:
int GetFilamentInfo(const std::string& VendorDirectory, json& pFilaList, const std::string& filepath,
std::string& sVendor, std::string& sType, std::unordered_set<std::string>& visiting);
GUI_App *m_MainPtr; GUI_App *m_MainPtr;
AppConfig m_appconfig_new; AppConfig m_appconfig_new;