Skip to content
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

[WIP] Julia 1.0.0 #19

Closed
wants to merge 4 commits into from
Closed

[WIP] Julia 1.0.0 #19

wants to merge 4 commits into from

Conversation

s-celles
Copy link
Contributor

It's building
It's also working for my use case
but don't merge before discussing about these changes

handle_error(ret, loc())
cts[]
end

# enum sp_return sp_set_config_cts(struct sp_port_config *config, enum sp_cts cts);
function sp_set_config_cts(config::Config, cts::SPcts)
ret = ccall((:sp_set_config_cts, libserialport), SPReturn,
(Config, SPcts), config, SPcts(cts))
(Config, Int32), config, Int(cts))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the kind of changes I have to do... but that I don't like

handle_error(ret, loc())
ret
end

# enum sp_return sp_set_dtr(struct sp_port *port, enum sp_dtr dtr);
function sp_set_dtr(port::Port, dtr::SPdtr)
ret = ccall((:sp_set_dtr, libserialport), SPReturn,
(Port, SPdtr), port, dtr)
(Port, Int32), port, Int(dtr))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here...

Should it be Int32, Int8, Int16, Int64...

I'm pretty sure that we should be more "generic", getting underlying type of SPdtr at compile time.

I also did:

Int(dtr)

but I should have done

Integer(dtr)

For your information

julia> typeof(Int(SP_DTR_ON))
Int64

julia> typeof(Integer(SP_DTR_ON))
Int32

@andrewadare
Copy link
Contributor

This is to address #17.

There are some changes that would be useful to include immediately, such as removing tic/toc, but I am less comfortable with other changes, such as casting the libserialport types to integers of various widths.

I'm going to leave this open for now, bringing in some of your changes manually (so this PR still definitely has value). We'd certainly like to be sure everything works properly with Julia 1.0 on 32 bit hosts such as a Raspberry Pi as well as on 64 bit systems.

@s-celles
Copy link
Contributor Author

What we need to avoid to smear code like I did, is to be able to get the underlying type of an Enum.
Maybe @tanmaykm could give us a tip on this?

@s-celles
Copy link
Contributor Author

@aviks could also help us on this

@andrewadare
Copy link
Contributor

What we need to avoid to smear code like I did, is to be able to get the underlying type of an Enum.

I think adding constructors for the Enums in 03a0ff5 was sufficient to deal with the deprecation warnings/errors.

@s-celles
Copy link
Contributor Author

Great idea! However I'm still wondering if the underlying type of an Enum can be known (for some others projects)

@andrewadare
Copy link
Contributor

I may not be understanding what you're trying to do, but according to https://docs.julialang.org/en/stable/base/base/#Special-Types-1, the base type is Int32 by default:

BaseType, which defaults to Int32, must be a primitive subtype of Integer. Member values can be converted between the enum type and BaseType. read and write perform these conversions automatically.

@andrewadare
Copy link
Contributor

This is kind of a silly example, but demonstrates the point:

using LibSerialPort

ret = SPReturn

for instance in instances(ret)
       println("$(typeof(instance)) $(supertype(typeof(instance)))")
end
SPReturn Enum{Int32}
SPReturn Enum{Int32}
SPReturn Enum{Int32}
SPReturn Enum{Int32}
SPReturn Enum{Int32}

@s-celles
Copy link
Contributor Author

s-celles commented Aug 19, 2018

@enum Fruit::UInt8 apple orange kiwi

is allowed but how to get UInt8 from Fruit?

@andrewadare
Copy link
Contributor

This is apparently one way:

julia> supertype(Fruit).parameters[1]
UInt8

@tanmaykm
Copy link
Member

Another way could be:

julia> Base.Enums.basetype(Fruit)
UInt8

@s-celles
Copy link
Contributor Author

Thanks @tanmaykm however I opened an issue at JuliaLang/julia#28778 because I think there is a room for improvement (at least for doc)

@s-celles s-celles closed this Aug 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants