cameleon-rs/cameleon

Formula parser does not handle name clash of variable names and special function names

Closed this issue · 6 comments

apriori commented

What did you expect to happen? What did actually happen?

In a vendor provided genicam.xml for the Point Grey/Flir Chameleon 3 CM3-U3-50S5M-CS there exist the following nodes:

<IntSwissKnife Name="ExposureCompRangeLocked_Int">
   <pVariable Name="EXP">ExposureCompLocked_Int</pVariable>
    <Formula>( EXP = 1 ) ? 0 : 1</Formula>
</IntSwissKnife>

The formula "( EXP = 1 ) ? 0 : 1" is meant to be interpreted with EXP being merely a variable. However, commit 902d843 parses this expression failing at line 494

cameleon/genapi/src/formula.rs

Lines 492 to 504 in 902d843

fn call(&mut self) -> Expr {
if let Some(op_kind) = self.next_call() {
self.expect(&Token::LParen);
let expr = self.expr();
self.expect(&Token::RParen);
Expr::UnOp {
kind: op_kind,
expr: expr.into(),
}
} else {
self.primary()
}
}

because the identifier EXP was determined as an UnOpKind::Exp.

How to reproduce the issue?

The easiest way would be to extend

cameleon/genapi/src/formula.rs

Lines 902 to 915 in 902d843

fn test_eval_with_env() {
let env = vec![
("VAR1", Expr::Integer(1)),
("EPS", Expr::Float(f64::EPSILON)),
]
.into_iter()
.collect();
test_eval_impl("ABS(SIN(PI / 2) - VAR1) < EPS", &env);
test_eval_impl("ABS(LN(E) - 1) < EPS", &env);
test_eval_impl("ABS(1. / 2. - 0.5) < EPS", &env);
test_eval_impl("ABS(2 ** -1 - 1. / 2.) < EPS", &env);
test_eval_impl("ABS(2 ** -1 ** 2 - 1. / 2.) < EPS", &env);
test_eval_impl("ABS(VAR1 + 1 / 4 - 1.25) < EPS", &env);

with a new test case, deliberately causing a name clash with either of the special functions, e.g.

#[test]
    fn test_eval_with_env() {
        let env = vec![
            ("VAR1", Expr::Integer(1)),
            ("EPS", Expr::Float(f64::EPSILON)),
            ("EXP", Expr::Float(1.0)),
        ]
        .into_iter()
        .collect();

        test_eval_impl("ABS(SIN(PI / 2) - VAR1) < EPS", &env);
        test_eval_impl("ABS(LN(E) - 1) < EPS", &env);
        test_eval_impl("ABS(1. / 2. - 0.5) < EPS", &env);
        test_eval_impl("ABS(2 ** -1 - 1. / 2.) < EPS", &env);
        test_eval_impl("ABS(2 ** -1 ** 2 - 1. / 2.) < EPS", &env);
        test_eval_impl("ABS(VAR1 + 1 / 4 - 1.25) < EPS", &env);
        test_eval_impl("( EXP = 1 ) ? 1 : 0", &env);
    }

Meta

Which manufacturer's camera did you use?

Point Grey/Flir

What is the model name of the camera?

Chameleon 3 CM3-U3-50S5M-CS
same issue also with: CM3-U3-28S4M
also assuming many more from this vendor.

Which Cameleon version are you using?

902d843

apriori commented

I was fidding around a bit, but things don't seem that easy for me.
The general problem is Parser::next_call() will attempt to detect "EXP" as a special function, However, at this time this is only correct, if there is Token::LParen immediately after it. One could assume a simple Lexer::peek() can answer this, but then at this time Parser::next_call() would have already already consumed the Token::Ident. ( Parser::next_call() would require a lookahead of 2 to easily answer this).
Simply storing that Token::Ident for a later "either or decision" might be an approach at first, but then, immediately after that one ends in up in trouble with the testcase:

""ABS(SIN(PI / 2) - VAR1) < EPS"

This test case expects Parser::next_call() to fail (and not consume the identifier PI, which is meant to be determined by Parser::primary() => Parser::next_float() .

Y-Nak commented

Thank you @apriori for opening a new issue and looking into it further.

To address this issue, you could insert if self.eat(&Token::LParen) {...} else { Expr::Ident(s) } to

cameleon/genapi/src/formula.rs

Lines 516 to 517 in 902d843

let s = self.next_ident().unwrap();
Expr::Ident(s)

If LParen is found right after an ident, all you need to do is converting the name to UnOpKind, parsing its argument, and eating RParen as

cameleon/genapi/src/formula.rs

Lines 495 to 500 in 902d843

let expr = self.expr();
self.expect(&Token::RParen);
Expr::UnOp {
kind: op_kind,
expr: expr.into(),
}
and

cameleon/genapi/src/formula.rs

Lines 536 to 554 in 902d843

let op = Some(match s.as_str() {
"NEG" => UnOpKind::Neg,
"SIN" => UnOpKind::Sin,
"COS" => UnOpKind::Cos,
"TAN" => UnOpKind::Tan,
"ASIN" => UnOpKind::Asin,
"ACOS" => UnOpKind::Acos,
"ATAN" => UnOpKind::Atan,
"ABS" => UnOpKind::Abs,
"EXP" => UnOpKind::Exp,
"LN" => UnOpKind::Ln,
"LG" => UnOpKind::Lg,
"SQRT" => UnOpKind::Sqrt,
"TRUNC" => UnOpKind::Trunc,
"FLOOR" => UnOpKind::Floor,
"CEIL" => UnOpKind::Ceil,
"ROUND" => UnOpKind::Round,
_ => return None,
});
do.

Then remove current Parser::call and Parser::next_call.

EndilWayfare commented

Had same problem on Windows 10 with a Point Grey Grasshopper GS3-U3-28S4M on current main.
Can confirm that apriori@30683dc makes the stream example work for me!

Testing with a Grasshopper GS3-U3-120S6M doesn't work, but it fails with "no camera found!" on both branches, so it seems to be an unrelated problem.

Y-Nak commented

Hi @EndilWayfare, thanks for the confirmation!

Testing with a Grasshopper GS3-U3-120S6M doesn't work, but it fails with "no camera found!" on both branches, so it seems to be an unrelated problem.

Let me check a few points.

  1. Did you run the example on windows, right? If so, have you installed the WinUSB driver correctly for the device as described here?
  2. If you have a Linux or macOS machine, could you try to run the example on it?
EndilWayfare commented

Hey @Y-Nak! First of all, allow me to say thank you so much for starting a GenICam implementation in Rust. When I was first planning out a project involving a camera ~1.5 months ago, I was disappointed to see that nothing native existed yet. I was afraid I'd have to use a crate that called out to a C library (not the end of the world) or figure out how to roll the FFI myself (not an area of expertise yet). Now that I'm starting the project in earnest, I'm glad I checked again and found your project!

Anyhow, back to the matter at hand, I am running on Windows and I did follow the instructions for WinUSB and Zadig. That's how I got the 28S4M working. I don't have a macOS machine, but I am using Windows Subsystem for Linux. Do you think that's sufficiently isolated from Windows? Or if I set up a Hyper-V or VirtualBox VM, would the USB pass-through allow Linux to take sufficient control?

I don't want to hijack this issue, just wanted to pop in and report success with another Point Grey camera as an additional datapoint. I'll file a new issue for my specific woes. :)

  • ❤️1
EndilWayfare commented

Ok, I raised and solved my issue in #109. Turns out I'm an idiot and I missed the composite device part. All the Point Gray cameras I have access to are now confirmed to work on the pull-request branch.

  • 👍1