-
Notifications
You must be signed in to change notification settings - Fork 4
Erb template #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Erb template #40
Changes from all commits
1582ed5
e95debc
2d35534
4301929
5739d8c
9548488
d23b7fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "erb" | ||
|
|
||
| module SDF | ||
| # Module for handling ERB files | ||
| module ERB | ||
| module_function | ||
|
|
||
|
Comment on lines
+8
to
+9
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid this. It's very easy to miss it when the module gets bigger. There are two better patterns:
The latter is also hard to miss, but the module in itself behaves normally (one can |
||
| # Parses an ERB string and returns the raw rendered string | ||
| # | ||
| # @param [String] erb_content ERB template file content as string | ||
| # @param [Hash] erb_args the configuration arguments to evaluate | ||
| # @return [String] the raw rendered XML string representing the model | ||
| def parse_erb_as_str(erb_content, **erb_args) | ||
| erb_engine = ::ERB.new(erb_content, trim_mode: "-") | ||
|
|
||
| # Render the ERB template with the passed hash arguments | ||
| erb_engine.result_with_hash(erb_args) | ||
| end | ||
|
|
||
| # Renders an ERB template and returns it as a REXML::Document | ||
| # | ||
| # @return [REXML::Document] the rendered sdf model | ||
| def render_erb_sdf_model(path, **erb_args) | ||
| erb_content = File.read(path) | ||
| solved_erb_as_sdf_str = parse_erb_as_str(erb_content, **erb_args) | ||
|
|
||
| REXML::Document.new(solved_erb_as_sdf_str) | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require_relative "erb" | ||
| require_relative "sdf_loader" | ||
|
|
||
| module SDF | ||
| # class to load SDF and ERB templated SDF files | ||
| class ERBLoader < Loader | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm missing the purpose or advantage of having separate loader and ERBLoader classes. This stuff is so simple, why not a single class ?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I remember correctly, @jhonasiv requested me to do it so we could enforce that when the |
||
| def initialize(erb_args: {}) | ||
| super() | ||
| @erb_args = erb_args | ||
| end | ||
|
|
||
| def parse_sdf_document(sdf_file) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you should make a difference between erb and non-erb files, that is parse ERB only when the extension is
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I told him to do it like this. If you mean for having an explicit error when a non-erb file is given to an ERBLoader, I feel this is overkill and it would painful in the Robot level to constantly juggle between loaders when the model file changes (I dont think you mean this, just getting it out there). In the case you want to split the functionality between
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @jhonasiv on this one, but in this case I would simply remove the base In case we enforce the files to end with But anyway, I don't have a strong opinion on this, so I would happily go with any |
||
| SDF::ERB.render_erb_sdf_model(sdf_file, **@erb_args) | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,24 @@ | ||
| module SDF | ||
| class InternalError < RuntimeError; end | ||
|
|
||
| module XML | ||
| # Exception raised when trying to load a model URI, but the model does | ||
| # not contain a SDF entry for the required SDF version | ||
| class UnavailableSDFVersionInModel < ArgumentError; end | ||
| # Exception raised when trying to load a file that is not a SDF file | ||
| class NotSDF < ArgumentError; end | ||
| # Exception raised when trying to load a malformed XML file | ||
| class InvalidXML < ArgumentError; end | ||
|
|
||
| # Exception raised when trying to resolve a model that cannot be found | ||
| # in {model_path} | ||
| class NoSuchModel < ArgumentError | ||
| attr_reader :model_name | ||
|
|
||
| def initialize(model_name) | ||
| super | ||
| @model_name = model_name | ||
| end | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| # frozen_string_literal: true | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SDF::Loader lives in |
||
|
|
||
| require_relative "erb" | ||
| require_relative "exceptions" | ||
|
|
||
| module SDF | ||
| # class to load SDF and ERB templated SDF files | ||
| class Loader | ||
| # Open a SDF file SDF file and returns its XML representation. | ||
| # | ||
| # @param [String] sdf_file the path to the SDF file | ||
| # @raise [Errno::ENOENT] if the files does not exist | ||
| # @raise [NotSDF] if the file is not a SDF file | ||
| # @raise [InvalidXML] if the file is not a valid XML file | ||
| # @return [REXML::Element] sdf_file's content as a REXML::Element instance | ||
| def load_sdf_raw(sdf_file) | ||
| find_or_raise_file_not_found(sdf_file) | ||
|
|
||
| sdf = begin | ||
| parse_sdf_document(sdf_file) | ||
| rescue REXML::ParseException => e | ||
| unless e.message.include?("No root") | ||
| raise SDF::XML::InvalidXML, | ||
| "Cannot load #{sdf_file}: #{e.message}" | ||
| end | ||
|
|
||
| REXML::Document.new | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know that it was in the original code, but ... I think the value of having an error message that says "it loads as XML but it has no root" rather than "this file has no root" does not warrant the complexity of having a codepath dedicated for it. Please simplify it. |
||
| end | ||
| validate_sdf_root(sdf, sdf_file) | ||
|
|
||
| sdf | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def find_or_raise_file_not_found(sdf_file) | ||
| return if File.exist?(sdf_file) | ||
|
|
||
| file_name = File.basename(sdf_file) | ||
| dir_path = File.dirname(sdf_file) | ||
| raise Errno::ENOENT, | ||
| "Cannot find '#{file_name}' in '#{dir_path}'." \ | ||
| "You probably want to update the GAZEBO_MODEL_PATH " \ | ||
| "environment variable, or set SDF.model_path explicitly." | ||
|
Comment on lines
+41
to
+44
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class knows nothing about GAZEBO_MODEL_PATH or SDF.model_path. It is given an already resolved path as argument. You should assume that the argument exists and let ENOENT propagate. The levels that do resolution based on GAZEBO_MODEL_PATH should error out if a model does not exist. |
||
| end | ||
|
|
||
| def parse_sdf_document(sdf_file) | ||
| File.open(sdf_file) do |io| | ||
| REXML::Document.new(io) | ||
| end | ||
| end | ||
|
|
||
| def validate_sdf_root(sdf, sdf_file) | ||
| unless sdf.root | ||
| raise SDF::XML::NotSDF, | ||
| "#{sdf_file} can be parsed as an XML file, but it " \ | ||
| "does not have a root" | ||
| end | ||
| return if %w[sdf gazebo].include?(sdf.root.name) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so: https://sdformat.org/spec/1.12/sdf/ |
||
|
|
||
| raise SDF::XML::NotSDF, "#{sdf_file} is not a SDF file" | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,7 @@ | ||
| require "rexml/document" | ||
| require_relative "exceptions" | ||
| require_relative "sdf_loader" | ||
| require_relative "erb_loader" | ||
|
Comment on lines
+2
to
+4
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do not use require_relative.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honest question, why is that? I thought require_relative was less error prune |
||
|
|
||
| module SDF | ||
| module XML | ||
|
|
@@ -7,24 +10,6 @@ module XML | |
| # (as version * 100, i.e. version 1.5 is represented by 150). Leave to | ||
| # nil to always read the latest. | ||
|
|
||
| # Exception raised when trying to load a model URI, but the model does | ||
| # not contain a SDF entry for the required SDF version | ||
| class UnavailableSDFVersionInModel < ArgumentError; end | ||
| # Exception raised when trying to load a file that is not a SDF file | ||
| class NotSDF < ArgumentError; end | ||
| # Exception raised when trying to load a malformed XML file | ||
| class InvalidXML < ArgumentError; end | ||
|
|
||
| # Exception raised when trying to resolve a model that cannot be found | ||
| # in {model_path} | ||
| class NoSuchModel < ArgumentError | ||
| attr_reader :model_name | ||
|
|
||
| def initialize(model_name) | ||
| @model_name = model_name | ||
| end | ||
| end | ||
|
|
||
| # The search path for models | ||
| # | ||
| # It defaults to GAZEBO_MODEL_PATH | ||
|
|
@@ -83,9 +68,9 @@ def self.clear_cache | |
| # SDF file for the required SDF version | ||
| # | ||
| # @return [REXML::Element] | ||
| def self.load_gazebo_model(dir, sdf_version = nil, metadata: false, flatten: true) | ||
| def self.load_gazebo_model(dir, sdf_version = nil, metadata: false, flatten: true, loader: SDF::Loader.new) | ||
| load_sdf(model_path_of(dir, sdf_version), metadata: metadata, | ||
| flatten: flatten) | ||
| flatten: flatten, loader: loader) | ||
| end | ||
|
|
||
| # Find model string into model.config path | ||
|
|
@@ -128,7 +113,7 @@ def self.model_path_of(dir, sdf_version = nil) | |
| # | ||
| # @!macro sdf_version | ||
| # @return [Hash<String,REXML::Element>] | ||
| def self.gazebo_models(sdf_version = nil) | ||
| def self.gazebo_models(sdf_version = nil, loader: SDF::ERBLoader.new) | ||
| @gazebo_models[sdf_version] ||= {} | ||
| @model_path.each do |p| | ||
| Dir.glob(File.join(p, "*")) do |subdir| | ||
|
|
@@ -140,7 +125,7 @@ def self.gazebo_models(sdf_version = nil) | |
| begin | ||
| sdf_file_path = model_path_of(subdir, sdf_version) | ||
| sdf, metadata = load_sdf(sdf_file_path, metadata: true, | ||
| flatten: false) | ||
| flatten: false, loader: loader) | ||
| @gazebo_models[sdf_version][File.basename(subdir)] = | ||
| ModelCacheEntry.new(sdf_file_path, sdf, metadata) | ||
| rescue UnavailableSDFVersionInModel | ||
|
|
@@ -165,7 +150,7 @@ def self.gazebo_models(sdf_version = nil) | |
| # @raise (see model_path_of) | ||
| # @raise [NoSuchModel] if the provided model name does not resolve to a | ||
| # model in {model_path} | ||
| # @return [REXML::Element] | ||
| # @return [String] the path to the SDF file for the model | ||
| def self.model_path_from_name(model_name, model_path: @model_path, sdf_version: nil) | ||
| @gazebo_models[sdf_version] ||= {} | ||
| cache = (@gazebo_models[sdf_version][model_name] ||= ModelCacheEntry.new) | ||
|
|
@@ -193,12 +178,12 @@ def self.model_path_from_name(model_name, model_path: @model_path, sdf_version: | |
| # model in {model_path} | ||
| # @return [REXML::Element] | ||
| def self.model_from_name( | ||
| model_name, sdf_version = nil, metadata: false, flatten: true | ||
| model_name, sdf_version = nil, metadata: false, flatten: true, loader: SDF::Loader.new | ||
| ) | ||
| path = model_path_from_name(model_name, sdf_version: sdf_version) | ||
| cache = @gazebo_models[sdf_version][model_name] | ||
| unless cache.xml | ||
| cache.xml, cache.metadata = load_sdf(path, metadata: true, flatten: false) | ||
| cache.xml, cache.metadata = load_sdf(path, metadata: true, flatten: false, loader: loader) | ||
| end | ||
| xml = cache.xml | ||
| if flatten | ||
|
|
@@ -213,6 +198,23 @@ def self.model_from_name( | |
| end | ||
| end | ||
|
|
||
| # Resolves relative paths and model:// URIs in the XML tree in-place | ||
| # | ||
| # This method traverses the XML tree starting from the given node, and | ||
| # expands any relative paths or `model://` URIs inside `<uri>` tags to | ||
| # absolute paths on the local filesystem. | ||
| # | ||
| # It skips `<include>` tags because those are resolved separately during | ||
| # {.add_include_tags}. | ||
| # | ||
| # @example Replaces a model:// mesh path: | ||
| # # Before: <uri>model://robot_model/hull.dae</uri> | ||
| # # After: <uri>/path/to/workspace/robot_models/models/sdf/robot_model/hull.dae</uri> | ||
| # | ||
| # @param [REXML::Element] node the XML element to traverse | ||
| # @!macro sdf_version | ||
| # @param [String] base_path the base directory path used to resolve relative paths | ||
| # @return [void] | ||
| def self.resolve_relative_uris(node, sdf_version, base_path) | ||
| nodes = [node] | ||
| until nodes.empty? | ||
|
|
@@ -264,16 +266,34 @@ def self.deep_copy_xml(node) | |
| # This method modifies the XML tree by replacing the include tags found | ||
| # as direct children of the provided element by the included content. | ||
| # | ||
| # @example | ||
| # # Before calling add_include_tags: | ||
| # # <world name="my_world"> | ||
| # # <include> | ||
| # # <uri>model://my_sensor</uri> | ||
| # # <name>custom_sensor</name> | ||
| # # <pose>1 0 0 0 0 0</pose> | ||
| # # </include> | ||
| # # </world> | ||
| # # | ||
| # # After calling add_include_tags: | ||
| # # <world name="my_world"> | ||
| # # <model name="custom_sensor"> | ||
| # # <pose>1 0 0 0 0 0</pose> | ||
| # # <link name="sensor_link">...</link> | ||
| # # </model> | ||
| # # </world> | ||
| # | ||
| # @param [REXML::Element] elem element to find include tags | ||
| # @!macro sdf_version | ||
| # @return [void] | ||
| def self.add_include_tags(elem, sdf_version, base_path) | ||
| def self.add_include_tags(elem, sdf_version, base_path, loader: SDF::Loader.new) | ||
| includes = {} | ||
|
|
||
| replacements = [] | ||
| elem.elements.each do |inc| | ||
| if inc.name == "world" || inc.name == "model" # model-within-model | ||
| added_includes = add_include_tags(inc, sdf_version, base_path) | ||
| added_includes = add_include_tags(inc, sdf_version, base_path, loader: loader) | ||
| includes.merge! added_includes do |_, old, new| | ||
| old + new | ||
| end | ||
|
|
@@ -309,11 +329,11 @@ def self.add_include_tags(elem, sdf_version, base_path) | |
|
|
||
| included_sdf, included_metadata = | ||
| model_from_name(model_name, sdf_version, metadata: true, | ||
| flatten: false) | ||
| flatten: false, loader: loader) | ||
| elsif File.directory?(uri_path = File.expand_path(uri, base_path)) | ||
| included_sdf, included_metadata = | ||
| load_gazebo_model(uri_path, sdf_version, metadata: true, | ||
| flatten: false) | ||
| flatten: false, loader: loader) | ||
| else | ||
| raise ArgumentError, | ||
| "URI #{uri} is neither a model:// URI nor an existing directory" | ||
|
|
@@ -362,39 +382,6 @@ def self.add_include_tags(elem, sdf_version, base_path) | |
| includes | ||
| end | ||
|
|
||
| # Open a SDF file and returns the XML representation | ||
| # | ||
| # Unlike {.load_sdf}, this really only loads the XML information, not | ||
| # resolving the include tags. | ||
| # | ||
| # @param [String] sdf_file the path to the SDF file | ||
| # @raise [Errno::ENOENT] if the files does not exist | ||
| # @raise [NotSDF] if the file is not a SDF file | ||
| # @raise [InvalidXML] if the file is not a valid XML file | ||
| # @return [REXML::Element] | ||
| def self.load_sdf_raw(sdf_file) | ||
| sdf = File.open(sdf_file) do |io| | ||
| REXML::Document.new(io) | ||
| rescue REXML::ParseException => e | ||
| unless e.message.match?(/No root/) | ||
| raise InvalidXML, "cannot load #{sdf_file}: #{e.message}" | ||
| end | ||
|
|
||
| REXML::Document.new | ||
| end | ||
|
|
||
| unless sdf.root | ||
| raise NotSDF, | ||
| "#{sdf_file} can be parsed as an XML file, but it does not have a root" | ||
| end | ||
|
|
||
| if sdf.root.name != "sdf" && sdf.root.name != "gazebo" | ||
| raise NotSDF, "#{sdf_file} is not a SDF file" | ||
| end | ||
|
|
||
| sdf | ||
| end | ||
|
|
||
| # Get sdf_version | ||
| # | ||
| # @param [REXML::Element] sdf element | ||
|
|
@@ -441,19 +428,22 @@ def self.sdf_version_of(sdf) | |
| # @param [Boolean] metadata whether the method should return a metadata hash | ||
| # about the various inclusions that have been performed. See above for | ||
| # the hash format | ||
| # @param [#load_sdf_raw] loader object that acts as a loader.Takes a file path as input | ||
| # and returns a REXML::Element with its content. Must respond to | ||
| # `load_sdf_raw(path: String) -> REXML::Element` | ||
| # @return [REXML::Element,(REXML::Element,Hash)] either the XML tree by itself | ||
| # if `metadata` is false, or the pair of the tree and the metadata hash | ||
| # otherwise. | ||
| # @raise [Errno::ENOENT] if the files does not exist | ||
| # @raise [NotSDF] if the file is not a SDF file | ||
| # @raise [InvalidXML] if the file is not a valid XML file | ||
| # @return [REXML::Element] | ||
| def self.load_sdf(sdf_file, flatten: true, metadata: false) | ||
| sdf = load_sdf_raw(sdf_file) | ||
| def self.load_sdf(sdf_file, flatten: true, metadata: false, loader: SDF::Loader.new) | ||
| sdf = loader.load_sdf_raw(sdf_file) | ||
| sdf_version = sdf_version_of(sdf) | ||
|
|
||
| sdf_metadata = Hash["includes" => {}, "path" => sdf_file] | ||
| includes = add_include_tags(sdf.root, sdf_version, File.dirname(sdf_file)) | ||
| includes = add_include_tags(sdf.root, sdf_version, File.dirname(sdf_file), loader: loader) | ||
| sdf_metadata["includes"].merge!(includes) do |_, old, new| | ||
| old + new | ||
| end | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please split code in modules only when you see that things get too big, or that you need the functionality in multiple places. Not when you think it/you will.
The functionality of erb.rb should really be straight into
ERBLoader#parse_sdf_documentUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will include it in the ERBLoader class.